From ab715a1e39014ec39cec256d43f7c999b63a4a16 Mon Sep 17 00:00:00 2001 From: olloz26 Date: Wed, 10 Jul 2024 15:52:01 +0200 Subject: [PATCH 01/12] feat: add custom openBIS type --- .../renku_data_services/storage/rclone.py | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/components/renku_data_services/storage/rclone.py b/components/renku_data_services/storage/rclone.py index 069b020b7..03875ef04 100644 --- a/components/renku_data_services/storage/rclone.py +++ b/components/renku_data_services/storage/rclone.py @@ -127,6 +127,57 @@ def __patch_schema_add_switch_provider(spec: list[dict[str, Any]]) -> None: ) existing_endpoint_spec["Provider"] += ",Switch" + @staticmethod + def __patch_schema_add_openbis_type(spec: list[dict[str, Any]]) -> None: + """Adds a fake type to help with setting up openBIS storage.""" + spec.append({ + "Name": "openbis", + "Description": "openBIS", + "Prefix": "openbis", + "Options": [ + { + "Name": "host", + "Help": "openBIS host to connect to.\n\nE.g. \"openbis-eln-lims.ethz.ch\".", + "Provider": "", + "Default": "", + "Value": None, + "ShortOpt": "", + "Hide": 0, + "Required": True, + "IsPassword": False, + "NoPrefix": False, + "Advanced": False, + "Exclusive": False, + "Sensitive": False, + "DefaultStr": "", + "ValueStr": "", + "Type": "string" + }, + { + "Name": "session_token", + "Help": "openBIS session token", + "Provider": "", + "Default": "", + "Value": None, + "ShortOpt": "", + "Hide": 0, + "Required": False, + "IsPassword": True, + "NoPrefix": False, + "Advanced": False, + "Exclusive": False, + "Sensitive": False, + "DefaultStr": "", + "ValueStr": "", + "Type": "string" + }, + ], + "CommandHelp": None, + "Aliases": None, + "Hide": False, + "MetadataInfo": None, + }) + @staticmethod def __patch_schema_remove_oauth_propeties(spec: list[dict[str, Any]]) -> None: """Removes OAuth2 fields since we can't do an oauth flow in the rclone CSI.""" From 60d722479a44e89794ec247fadefe759d77886cb Mon Sep 17 00:00:00 2001 From: olloz26 Date: Thu, 11 Jul 2024 15:59:18 +0200 Subject: [PATCH 02/12] style: fix --- .../renku_data_services/storage/rclone.py | 96 ++++++++++--------- 1 file changed, 49 insertions(+), 47 deletions(-) diff --git a/components/renku_data_services/storage/rclone.py b/components/renku_data_services/storage/rclone.py index 03875ef04..3ec2bb465 100644 --- a/components/renku_data_services/storage/rclone.py +++ b/components/renku_data_services/storage/rclone.py @@ -130,53 +130,55 @@ def __patch_schema_add_switch_provider(spec: list[dict[str, Any]]) -> None: @staticmethod def __patch_schema_add_openbis_type(spec: list[dict[str, Any]]) -> None: """Adds a fake type to help with setting up openBIS storage.""" - spec.append({ - "Name": "openbis", - "Description": "openBIS", - "Prefix": "openbis", - "Options": [ - { - "Name": "host", - "Help": "openBIS host to connect to.\n\nE.g. \"openbis-eln-lims.ethz.ch\".", - "Provider": "", - "Default": "", - "Value": None, - "ShortOpt": "", - "Hide": 0, - "Required": True, - "IsPassword": False, - "NoPrefix": False, - "Advanced": False, - "Exclusive": False, - "Sensitive": False, - "DefaultStr": "", - "ValueStr": "", - "Type": "string" - }, - { - "Name": "session_token", - "Help": "openBIS session token", - "Provider": "", - "Default": "", - "Value": None, - "ShortOpt": "", - "Hide": 0, - "Required": False, - "IsPassword": True, - "NoPrefix": False, - "Advanced": False, - "Exclusive": False, - "Sensitive": False, - "DefaultStr": "", - "ValueStr": "", - "Type": "string" - }, - ], - "CommandHelp": None, - "Aliases": None, - "Hide": False, - "MetadataInfo": None, - }) + spec.append( + { + "Name": "openbis", + "Description": "openBIS", + "Prefix": "openbis", + "Options": [ + { + "Name": "host", + "Help": 'openBIS host to connect to.\n\nE.g. "openbis-eln-lims.ethz.ch".', + "Provider": "", + "Default": "", + "Value": None, + "ShortOpt": "", + "Hide": 0, + "Required": True, + "IsPassword": False, + "NoPrefix": False, + "Advanced": False, + "Exclusive": False, + "Sensitive": False, + "DefaultStr": "", + "ValueStr": "", + "Type": "string", + }, + { + "Name": "session_token", + "Help": "openBIS session token", + "Provider": "", + "Default": "", + "Value": None, + "ShortOpt": "", + "Hide": 0, + "Required": False, + "IsPassword": True, + "NoPrefix": False, + "Advanced": False, + "Exclusive": False, + "Sensitive": False, + "DefaultStr": "", + "ValueStr": "", + "Type": "string", + }, + ], + "CommandHelp": None, + "Aliases": None, + "Hide": False, + "MetadataInfo": None, + } + ) @staticmethod def __patch_schema_remove_oauth_propeties(spec: list[dict[str, Any]]) -> None: From 35c83aee82efac678aea9737684cbf91e7950492 Mon Sep 17 00:00:00 2001 From: olloz26 Date: Thu, 11 Jul 2024 18:21:09 +0200 Subject: [PATCH 03/12] feat: add openBIS PAT request tool --- components/renku_data_services/utils/core.py | 52 ++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/components/renku_data_services/utils/core.py b/components/renku_data_services/utils/core.py index 4d3362bd4..b25418d97 100644 --- a/components/renku_data_services/utils/core.py +++ b/components/renku_data_services/utils/core.py @@ -1,6 +1,7 @@ """Shared utility functions.""" import functools +import datetime import os import ssl from collections.abc import Awaitable, Callable @@ -90,3 +91,54 @@ async def transaction_wrapper(self: _WithSessionMaker, *args: _P.args, **kwargs: return await f(self, *args, **kwargs) return transaction_wrapper + + +async def get_openbis_pat( + host: str, + session_id: str, + personal_access_token_session_name: str = "renku", + minimum_validity_in_days: int = 2, + timeout: int = 12, +) -> str: + url = f"https://{host}/openbis/openbis/rmi-application-server-v3.json" + + get_server_information = {"method": "getServerInformation", "params": [session_id], "id": "2", "jsonrpc": "2.0"} + + async with httpx.AsyncClient(verify=get_ssl_context()) as client: + response = await client.post(url, json=get_server_information, timeout=timeout) + if response.status_code == 200: + json1: dict[str, dict[str, str]] = response.json() + personal_access_tokens_max_validity_period = int( + json1["result"]["personal-access-tokens-max-validity-period"] + ) + + valid_from = datetime.datetime.now() + valid_to = valid_from + datetime.timedelta(seconds=personal_access_tokens_max_validity_period) + validity_in_days = (valid_to - valid_from).days + if validity_in_days >= minimum_validity_in_days: + create_personal_access_tokens = { + "method": "createPersonalAccessTokens", + "params": [ + session_id, + { + "@type": "as.dto.pat.create.PersonalAccessTokenCreation", + "sessionName": personal_access_token_session_name, + "validFromDate": int(valid_from.timestamp() * 1000), + "validToDate": int(valid_to.timestamp() * 1000), + }, + ], + "id": "2", + "jsonrpc": "2.0", + } + + response = await client.post(url, json=create_personal_access_tokens, timeout=timeout) + + if response.status_code == 200: + json2: dict[str, list[dict[str, str]]] = response.json() + return json2["result"][0]["permId"] + else: + raise Exception( + f"The maximum allowed validity period of a personal access token is less than {minimum_validity_in_days} days." + ) + + raise Exception("An openBIS personal access token related request failed.") From 84cf5bba1f587dafb1d88bda7aadddf25444b071 Mon Sep 17 00:00:00 2001 From: olloz26 Date: Thu, 11 Jul 2024 18:42:10 +0200 Subject: [PATCH 04/12] style: fix --- components/renku_data_services/utils/core.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/components/renku_data_services/utils/core.py b/components/renku_data_services/utils/core.py index b25418d97..aa57eddaa 100644 --- a/components/renku_data_services/utils/core.py +++ b/components/renku_data_services/utils/core.py @@ -1,7 +1,7 @@ """Shared utility functions.""" -import functools import datetime +import functools import os import ssl from collections.abc import Awaitable, Callable @@ -100,6 +100,7 @@ async def get_openbis_pat( minimum_validity_in_days: int = 2, timeout: int = 12, ) -> str: + """Requests an openBIS PAT with an openBIS session ID.""" url = f"https://{host}/openbis/openbis/rmi-application-server-v3.json" get_server_information = {"method": "getServerInformation", "params": [session_id], "id": "2", "jsonrpc": "2.0"} @@ -138,7 +139,8 @@ async def get_openbis_pat( return json2["result"][0]["permId"] else: raise Exception( - f"The maximum allowed validity period of a personal access token is less than {minimum_validity_in_days} days." + "The maximum allowed validity period of a personal access token is less than " + f"{minimum_validity_in_days} days." ) raise Exception("An openBIS personal access token related request failed.") From b2706bc740ed1fa7823cc72b838ef57128ef6c64 Mon Sep 17 00:00:00 2001 From: olloz26 Date: Wed, 31 Jul 2024 13:25:22 +0200 Subject: [PATCH 05/12] fix: correct a property value --- components/renku_data_services/storage/rclone.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/renku_data_services/storage/rclone.py b/components/renku_data_services/storage/rclone.py index 3ec2bb465..efd4af0e2 100644 --- a/components/renku_data_services/storage/rclone.py +++ b/components/renku_data_services/storage/rclone.py @@ -167,7 +167,7 @@ def __patch_schema_add_openbis_type(spec: list[dict[str, Any]]) -> None: "NoPrefix": False, "Advanced": False, "Exclusive": False, - "Sensitive": False, + "Sensitive": True, "DefaultStr": "", "ValueStr": "", "Type": "string", From 0bb9cf8a9bb2e8ce071b65f3a382da0d49afd8ac Mon Sep 17 00:00:00 2001 From: olloz26 Date: Thu, 22 Aug 2024 16:12:33 +0200 Subject: [PATCH 06/12] feat: add expiration timestamps for secrets --- DEVELOPING.md | 6 +- ...829ed2f_add_secret_expiration_timestamp.py | 30 +++++ components/renku_data_services/secrets/db.py | 42 ++++-- .../renku_data_services/secrets/models.py | 1 + components/renku_data_services/secrets/orm.py | 10 +- .../renku_data_services/storage/rclone.py | 4 +- .../renku_data_services/users/api.spec.yaml | 22 +++- .../renku_data_services/users/apispec.py | 17 ++- .../renku_data_services/users/blueprints.py | 31 +++-- components/renku_data_services/utils/core.py | 89 +++++++------ .../data_api/test_secret.py | 122 ++++++++++++++---- .../data_api/test_storage_v2.py | 0 test/conftest.py | 12 ++ 13 files changed, 296 insertions(+), 90 deletions(-) create mode 100644 components/renku_data_services/migrations/versions/7bc32829ed2f_add_secret_expiration_timestamp.py create mode 100644 test/bases/renku_data_services/data_api/test_storage_v2.py diff --git a/DEVELOPING.md b/DEVELOPING.md index 55e7616b3..059189296 100644 --- a/DEVELOPING.md +++ b/DEVELOPING.md @@ -115,8 +115,10 @@ function if you prefer to keep your favorite shell. ## Running Tests You can run style checks using `make style_checks`. -To run the test test suite, use `make tests` (you likely need to run in the devcontainer for this to work, as it needs -some surrounding services to run). +To run the test suite, use `make tests` (you likely need to run in the devcontainer for this to work, as it needs some +surrounding services to run). +* Run a specific test e.g.: `poetry run pytest test/bases/renku_data_services/data_api/test_storage_v2.py::test_storage_v2_create_openbis_secret` +* Also run tests marked with `@pytest.mark.myskip`: `PYTEST_FORCE_RUN_MYSKIPS=1 make tests` ## Migrations diff --git a/components/renku_data_services/migrations/versions/7bc32829ed2f_add_secret_expiration_timestamp.py b/components/renku_data_services/migrations/versions/7bc32829ed2f_add_secret_expiration_timestamp.py new file mode 100644 index 000000000..0812b2adb --- /dev/null +++ b/components/renku_data_services/migrations/versions/7bc32829ed2f_add_secret_expiration_timestamp.py @@ -0,0 +1,30 @@ +"""add_secret_expiration_timestamp + +Revision ID: 7bc32829ed2f +Revises: 9058bf0a1a12 +Create Date: 2024-08-21 12:38:30.932694 + +""" + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision = "7bc32829ed2f" +down_revision = "9058bf0a1a12" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.add_column( + "secrets", sa.Column("expiration_timestamp", sa.DateTime(timezone=True), nullable=True), schema="secrets" + ) + # ### end Alembic commands ### + + +def downgrade() -> None: + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column("secrets", "expiration_timestamp", schema="secrets") + # ### end Alembic commands ### diff --git a/components/renku_data_services/secrets/db.py b/components/renku_data_services/secrets/db.py index 2fdf8fe4e..45f32d3e9 100644 --- a/components/renku_data_services/secrets/db.py +++ b/components/renku_data_services/secrets/db.py @@ -1,10 +1,10 @@ """Database repo for secrets.""" from collections.abc import AsyncGenerator, Callable, Sequence -from datetime import UTC, datetime +from datetime import UTC, datetime, timedelta from typing import cast -from sqlalchemy import delete, select +from sqlalchemy import Select, delete, or_, select from sqlalchemy.exc import IntegrityError from sqlalchemy.ext.asyncio import AsyncSession from ulid import ULID @@ -25,11 +25,23 @@ def __init__( ) -> None: self.session_maker = session_maker + def _get_stmt(self, requested_by: APIUser) -> Select[tuple[SecretORM]]: + return ( + select(SecretORM) + .where(SecretORM.user_id == requested_by.id) + .where( + or_( + SecretORM.expiration_timestamp.is_(None), + SecretORM.expiration_timestamp > datetime.now(UTC) + timedelta(seconds=120), + ) + ) + ) + @only_authenticated async def get_user_secrets(self, requested_by: APIUser, kind: SecretKind) -> list[Secret]: """Get all user's secrets from the database.""" async with self.session_maker() as session: - stmt = select(SecretORM).where(SecretORM.user_id == requested_by.id).where(SecretORM.kind == kind) + stmt = self._get_stmt(requested_by).where(SecretORM.kind == kind) res = await session.execute(stmt) orm = res.scalars().all() return [o.dump() for o in orm] @@ -38,7 +50,7 @@ async def get_user_secrets(self, requested_by: APIUser, kind: SecretKind) -> lis async def get_secret_by_id(self, requested_by: APIUser, secret_id: ULID) -> Secret | None: """Get a specific user secret from the database.""" async with self.session_maker() as session: - stmt = select(SecretORM).where(SecretORM.user_id == requested_by.id).where(SecretORM.id == secret_id) + stmt = self._get_stmt(requested_by).where(SecretORM.id == secret_id) res = await session.execute(stmt) orm = res.scalar_one_or_none() if orm is None: @@ -66,6 +78,7 @@ async def insert_secret(self, requested_by: APIUser, secret: UnsavedSecret) -> S encrypted_value=secret.encrypted_value, encrypted_key=secret.encrypted_key, kind=secret.kind, + expiration_timestamp=secret.expiration_timestamp, ) session.add(orm) @@ -83,19 +96,26 @@ async def insert_secret(self, requested_by: APIUser, secret: UnsavedSecret) -> S @only_authenticated async def update_secret( - self, requested_by: APIUser, secret_id: ULID, encrypted_value: bytes, encrypted_key: bytes + self, + requested_by: APIUser, + secret_id: ULID, + encrypted_value: bytes, + encrypted_key: bytes, + expiration_timestamp: datetime | None, ) -> Secret: """Update a secret.""" async with self.session_maker() as session, session.begin(): - result = await session.execute( - select(SecretORM).where(SecretORM.id == secret_id).where(SecretORM.user_id == requested_by.id) - ) + result = await session.execute(self._get_stmt(requested_by).where(SecretORM.id == secret_id)) secret = result.scalar_one_or_none() if secret is None: raise errors.MissingResourceError(message=f"The secret with id '{secret_id}' cannot be found") - secret.update(encrypted_value=encrypted_value, encrypted_key=encrypted_key) + secret.update( + encrypted_value=encrypted_value, + encrypted_key=encrypted_key, + expiration_timestamp=expiration_timestamp, + ) return secret.dump() @only_authenticated @@ -103,9 +123,7 @@ async def delete_secret(self, requested_by: APIUser, secret_id: ULID) -> None: """Delete a secret.""" async with self.session_maker() as session, session.begin(): - result = await session.execute( - select(SecretORM).where(SecretORM.id == secret_id).where(SecretORM.user_id == requested_by.id) - ) + result = await session.execute(self._get_stmt(requested_by).where(SecretORM.id == secret_id)) secret = result.scalar_one_or_none() if secret is None: return None diff --git a/components/renku_data_services/secrets/models.py b/components/renku_data_services/secrets/models.py index cc7d46e8f..491a25d01 100644 --- a/components/renku_data_services/secrets/models.py +++ b/components/renku_data_services/secrets/models.py @@ -24,6 +24,7 @@ class UnsavedSecret(BaseModel): encrypted_key: bytes = Field(repr=False) modification_date: datetime = Field(default_factory=lambda: datetime.now(UTC).replace(microsecond=0), init=False) kind: SecretKind + expiration_timestamp: datetime | None = Field(default=None) class Secret(UnsavedSecret): diff --git a/components/renku_data_services/secrets/orm.py b/components/renku_data_services/secrets/orm.py index 5f82d94a1..6b872dc94 100644 --- a/components/renku_data_services/secrets/orm.py +++ b/components/renku_data_services/secrets/orm.py @@ -35,6 +35,9 @@ class SecretORM(BaseORM): encrypted_value: Mapped[bytes] = mapped_column(LargeBinary()) encrypted_key: Mapped[bytes] = mapped_column(LargeBinary()) kind: Mapped[models.SecretKind] + expiration_timestamp: Mapped[Optional[datetime]] = mapped_column( + "expiration_timestamp", DateTime(timezone=True), default=None, nullable=True + ) modification_date: Mapped[datetime] = mapped_column( "modification_date", DateTime(timezone=True), default_factory=lambda: datetime.now(UTC).replace(microsecond=0) ) @@ -51,6 +54,7 @@ def dump(self) -> models.Secret: encrypted_value=self.encrypted_value, encrypted_key=self.encrypted_key, kind=self.kind, + expiration_timestamp=self.expiration_timestamp, ) secret.modification_date = self.modification_date return secret @@ -62,12 +66,14 @@ def load(cls, secret: models.UnsavedSecret) -> "SecretORM": name=secret.name, encrypted_value=secret.encrypted_value, encrypted_key=secret.encrypted_key, - modification_date=secret.modification_date, kind=secret.kind, + expiration_timestamp=secret.expiration_timestamp, + modification_date=secret.modification_date, ) - def update(self, encrypted_value: bytes, encrypted_key: bytes) -> None: + def update(self, encrypted_value: bytes, encrypted_key: bytes, expiration_timestamp: datetime | None) -> None: """Update an existing secret.""" self.encrypted_value = encrypted_value self.encrypted_key = encrypted_key + self.expiration_timestamp = expiration_timestamp self.modification_date = datetime.now(UTC).replace(microsecond=0) diff --git a/components/renku_data_services/storage/rclone.py b/components/renku_data_services/storage/rclone.py index efd4af0e2..2a5729f8a 100644 --- a/components/renku_data_services/storage/rclone.py +++ b/components/renku_data_services/storage/rclone.py @@ -162,7 +162,7 @@ def __patch_schema_add_openbis_type(spec: list[dict[str, Any]]) -> None: "Value": None, "ShortOpt": "", "Hide": 0, - "Required": False, + "Required": True, "IsPassword": True, "NoPrefix": False, "Advanced": False, @@ -572,7 +572,7 @@ class RCloneProviderSchema(BaseModel): @property def required_options(self) -> list[RCloneOption]: """Returns all required options for this provider.""" - return [o for o in self.options if o.required] + return [o for o in self.options if o.required and not o.sensitive] @property def sensitive_options(self) -> list[RCloneOption]: diff --git a/components/renku_data_services/users/api.spec.yaml b/components/renku_data_services/users/api.spec.yaml index 3bb060852..db688b719 100644 --- a/components/renku_data_services/users/api.spec.yaml +++ b/components/renku_data_services/users/api.spec.yaml @@ -418,20 +418,23 @@ components: $ref: "#/components/schemas/Ulid" name: $ref: "#/components/schemas/SecretName" - modification_date: - $ref: "#/components/schemas/ModificationDate" kind: $ref: "#/components/schemas/SecretKind" + expiration_timestamp: + $ref: "#/components/schemas/ExpirationTimestamp" + modification_date: + $ref: "#/components/schemas/ModificationDate" required: - "id" - "name" - - "modification_date" - "kind" + - "modification_date" example: id: "01AN4Z79ZS5XN0F25N3DB94T4R" name: "S3-Credentials" - modification_date: "2024-01-16T11:42:05Z" kind: general + expiration_timestamp: null + modification_date: "2024-01-16T11:42:05Z" SecretPost: description: Secret metadata to be created type: object @@ -446,6 +449,8 @@ components: - $ref: "#/components/schemas/SecretKind" - default: "general" default: general + expiration_timestamp: + $ref: "#/components/schemas/ExpirationTimestamp" required: - "name" - "value" @@ -456,6 +461,8 @@ components: properties: value: $ref: "#/components/schemas/SecretValue" + expiration_timestamp: + $ref: "#/components/schemas/ExpirationTimestamp" required: - "value" SecretName: @@ -487,6 +494,13 @@ components: enum: - general - storage + ExpirationTimestamp: + description: The date and time the secret is not valid anymore (this is in any timezone) + type: string + nullable: true + format: date-time + example: "2030-11-01T17:32:28UTC+01:00" + default: null UserPreferences: type: object description: The object containing user preferences diff --git a/components/renku_data_services/users/apispec.py b/components/renku_data_services/users/apispec.py index 5e0637b51..01cf4be08 100644 --- a/components/renku_data_services/users/apispec.py +++ b/components/renku_data_services/users/apispec.py @@ -198,12 +198,17 @@ class SecretWithId(BaseAPISpec): min_length=1, pattern="^[a-zA-Z0-9_\\-.]*$", ) + kind: SecretKind + expiration_timestamp: Optional[datetime] = Field( + None, + description="The date and time the secret is not valid anymore (this is in any timezone)", + example="2030-11-01T17:32:28UTC+01:00", + ) modification_date: datetime = Field( ..., description="The date and time the secret was created or modified (this is always in UTC)", example="2023-11-01T17:32:28Z", ) - kind: SecretKind class SecretPost(BaseAPISpec): @@ -225,6 +230,11 @@ class SecretPost(BaseAPISpec): min_length=1, ) kind: SecretKind = SecretKind.general + expiration_timestamp: Optional[datetime] = Field( + None, + description="The date and time the secret is not valid anymore (this is in any timezone)", + example="2030-11-01T17:32:28UTC+01:00", + ) class SecretPatch(BaseAPISpec): @@ -237,6 +247,11 @@ class SecretPatch(BaseAPISpec): max_length=5000, min_length=1, ) + expiration_timestamp: Optional[datetime] = Field( + None, + description="The date and time the secret is not valid anymore (this is in any timezone)", + example="2030-11-01T17:32:28UTC+01:00", + ) class PinnedProjects(BaseAPISpec): diff --git a/components/renku_data_services/users/blueprints.py b/components/renku_data_services/users/blueprints.py index 74dd7971f..ad8a48e48 100644 --- a/components/renku_data_services/users/blueprints.py +++ b/components/renku_data_services/users/blueprints.py @@ -152,7 +152,11 @@ async def _get_all( secret_kind = SecretKind[query.kind.value] secrets = await self.secret_repo.get_user_secrets(requested_by=user, kind=secret_kind) secrets_json = [ - secret.model_dump(include={"name", "id", "modification_date", "kind"}, exclude_none=True, mode="json") + secret.model_dump( + include={"id", "name", "kind", "expiration_timestamp", "modification_date"}, + exclude_none=True, + mode="json", + ) for secret in secrets ] return validated_json( @@ -173,9 +177,11 @@ async def _get_one(_: Request, user: base_models.APIUser, secret_id: ULID) -> JS if not secret: raise errors.MissingResourceError(message=f"The secret with id {secret_id} cannot be found.") result = secret.model_dump( - include={"name", "id", "modification_date", "kind"}, exclude_none=True, mode="json" + include={"id", "name", "kind", "expiration_timestamp", "modification_date"}, + exclude_none=False, + mode="json", ) - return validated_json(apispec.SecretWithId, result) + return validated_json(apispec.SecretWithId, result, exclude_none=False) return "/user/secrets/", ["GET"], _get_one @@ -197,12 +203,15 @@ async def _post(_: Request, user: base_models.APIUser, body: apispec.SecretPost) encrypted_value=encrypted_value, encrypted_key=encrypted_key, kind=SecretKind[body.kind.value], + expiration_timestamp=body.expiration_timestamp, ) inserted_secret = await self.secret_repo.insert_secret(requested_by=user, secret=secret) result = inserted_secret.model_dump( - include={"name", "id", "modification_date", "kind"}, exclude_none=True, mode="json" + include={"id", "name", "kind", "expiration_timestamp", "modification_date"}, + exclude_none=False, + mode="json", ) - return validated_json(apispec.SecretWithId, result, 201) + return validated_json(apispec.SecretWithId, result, 201, exclude_none=False) return "/user/secrets", ["POST"], _post @@ -222,13 +231,19 @@ async def _patch( secret_value=body.value, ) updated_secret = await self.secret_repo.update_secret( - requested_by=user, secret_id=secret_id, encrypted_value=encrypted_value, encrypted_key=encrypted_key + requested_by=user, + secret_id=secret_id, + encrypted_value=encrypted_value, + encrypted_key=encrypted_key, + expiration_timestamp=body.expiration_timestamp, ) result = updated_secret.model_dump( - include={"name", "id", "modification_date", "kind"}, exclude_none=True, mode="json" + include={"id", "name", "kind", "expiration_timestamp", "modification_date"}, + exclude_none=False, + mode="json", ) - return validated_json(apispec.SecretWithId, result) + return validated_json(apispec.SecretWithId, result, exclude_none=False) return "/user/secrets/", ["PATCH"], _patch diff --git a/components/renku_data_services/utils/core.py b/components/renku_data_services/utils/core.py index aa57eddaa..536a23eb9 100644 --- a/components/renku_data_services/utils/core.py +++ b/components/renku_data_services/utils/core.py @@ -1,10 +1,10 @@ """Shared utility functions.""" -import datetime import functools import os import ssl from collections.abc import Awaitable, Callable +from datetime import datetime, timedelta from typing import Any, Concatenate, ParamSpec, Protocol, TypeVar, cast import httpx @@ -93,54 +93,69 @@ async def transaction_wrapper(self: _WithSessionMaker, *args: _P.args, **kwargs: return transaction_wrapper +def _get_url(host: str) -> str: + return f"https://{host}/openbis/openbis/rmi-application-server-v3.json" + + +async def get_openbis_session_token( + host: str, + username: str, + password: str, + timeout: int = 12, +) -> str: + """Requests an openBIS session token with the user's login credentials.""" + login = {"method": "login", "params": [username, password], "id": "2", "jsonrpc": "2.0"} + async with httpx.AsyncClient(verify=get_ssl_context()) as client: + response = await client.post(_get_url(host), json=login, timeout=timeout) + json: dict[str, str] = response.json() + return json["result"] + + async def get_openbis_pat( host: str, session_id: str, personal_access_token_session_name: str = "renku", minimum_validity_in_days: int = 2, timeout: int = 12, -) -> str: +) -> tuple[str, datetime]: """Requests an openBIS PAT with an openBIS session ID.""" - url = f"https://{host}/openbis/openbis/rmi-application-server-v3.json" - - get_server_information = {"method": "getServerInformation", "params": [session_id], "id": "2", "jsonrpc": "2.0"} + url = _get_url(host) async with httpx.AsyncClient(verify=get_ssl_context()) as client: + get_server_information = {"method": "getServerInformation", "params": [session_id], "id": "2", "jsonrpc": "2.0"} response = await client.post(url, json=get_server_information, timeout=timeout) if response.status_code == 200: json1: dict[str, dict[str, str]] = response.json() - personal_access_tokens_max_validity_period = int( - json1["result"]["personal-access-tokens-max-validity-period"] - ) - - valid_from = datetime.datetime.now() - valid_to = valid_from + datetime.timedelta(seconds=personal_access_tokens_max_validity_period) - validity_in_days = (valid_to - valid_from).days - if validity_in_days >= minimum_validity_in_days: - create_personal_access_tokens = { - "method": "createPersonalAccessTokens", - "params": [ - session_id, - { - "@type": "as.dto.pat.create.PersonalAccessTokenCreation", - "sessionName": personal_access_token_session_name, - "validFromDate": int(valid_from.timestamp() * 1000), - "validToDate": int(valid_to.timestamp() * 1000), - }, - ], - "id": "2", - "jsonrpc": "2.0", - } - - response = await client.post(url, json=create_personal_access_tokens, timeout=timeout) - - if response.status_code == 200: - json2: dict[str, list[dict[str, str]]] = response.json() - return json2["result"][0]["permId"] - else: - raise Exception( - "The maximum allowed validity period of a personal access token is less than " - f"{minimum_validity_in_days} days." + if "error" not in json1: + personal_access_tokens_max_validity_period = int( + json1["result"]["personal-access-tokens-max-validity-period"] ) + valid_from = datetime.now() + valid_to = valid_from + timedelta(seconds=personal_access_tokens_max_validity_period) + validity_in_days = (valid_to - valid_from).days + if validity_in_days >= minimum_validity_in_days: + create_personal_access_tokens = { + "method": "createPersonalAccessTokens", + "params": [ + session_id, + { + "@type": "as.dto.pat.create.PersonalAccessTokenCreation", + "sessionName": personal_access_token_session_name, + "validFromDate": int(valid_from.timestamp() * 1000), + "validToDate": int(valid_to.timestamp() * 1000), + }, + ], + "id": "2", + "jsonrpc": "2.0", + } + response = await client.post(url, json=create_personal_access_tokens, timeout=timeout) + if response.status_code == 200: + json2: dict[str, list[dict[str, str]]] = response.json() + return json2["result"][0]["permId"], valid_to + else: + raise Exception( + "The maximum allowed validity period of a personal access token is less than " + f"{minimum_validity_in_days} days." + ) raise Exception("An openBIS personal access token related request failed.") diff --git a/test/bases/renku_data_services/data_api/test_secret.py b/test/bases/renku_data_services/data_api/test_secret.py index c4b132ab9..e47304247 100644 --- a/test/bases/renku_data_services/data_api/test_secret.py +++ b/test/bases/renku_data_services/data_api/test_secret.py @@ -1,6 +1,8 @@ """Tests for secrets blueprints.""" +import time from base64 import b64decode +from datetime import datetime, timedelta from typing import Any import pytest @@ -23,8 +25,10 @@ @pytest.fixture def create_secret(sanic_client: SanicASGITestClient, user_headers): - async def create_secret_helper(name: str, value: str, kind: str = "general") -> dict[str, Any]: - payload = {"name": name, "value": value, "kind": kind} + async def create_secret_helper( + name: str, value: str, kind: str = "general", expiration_timestamp: str = None + ) -> dict[str, Any]: + payload = {"name": name, "value": value, "kind": kind, "expiration_timestamp": expiration_timestamp} _, response = await sanic_client.post("/api/data/user/secrets", headers=user_headers, json=payload) @@ -46,11 +50,32 @@ async def test_create_secrets(sanic_client: SanicASGITestClient, user_headers, k assert response.status_code == 201, response.text assert response.json is not None - assert response.json.keys() == {"name", "id", "modification_date", "kind"} + assert response.json.keys() == {"id", "name", "kind", "expiration_timestamp", "modification_date"} assert response.json["name"] == "my-secret" assert response.json["id"] is not None + assert response.json["kind"] == kind + assert response.json["expiration_timestamp"] is None assert response.json["modification_date"] is not None + + +@pytest.mark.asyncio +@pytest.mark.parametrize("kind", [e.value for e in apispec.SecretKind]) +async def test_create_secrets_with_expiration_timestamps(sanic_client: SanicASGITestClient, user_headers, kind) -> None: + payload = { + "name": "my-secret-that-expires", + "value": "42", + "kind": kind, + "expiration_timestamp": "2029-12-31T23:59:59+01:00", + } + _, response = await sanic_client.post("/api/data/user/secrets", headers=user_headers, json=payload) + assert response.status_code == 201, response.text + assert response.json is not None + assert response.json.keys() == {"id", "name", "kind", "expiration_timestamp", "modification_date"} + assert response.json["name"] == "my-secret-that-expires" + assert response.json["id"] is not None assert response.json["kind"] == kind + assert response.json["expiration_timestamp"] == "2029-12-31T23:59:59+01:00" + assert response.json["modification_date"] is not None @pytest.mark.asyncio @@ -59,15 +84,36 @@ async def test_get_one_secret(sanic_client: SanicASGITestClient, user_headers, c secret = await create_secret("secret-2", "value-2") await create_secret("secret-3", "value-3") - secret_id = secret["id"] + _, response = await sanic_client.get(f"/api/data/user/secrets/{secret["id"]}", headers=user_headers) + assert response.status_code == 200, response.text + assert response.json is not None + assert response.json["name"] == secret["name"] + assert response.json["id"] == secret["id"] + assert "value" not in response.json + + +@pytest.mark.asyncio +async def test_get_one_secret_not_expired(sanic_client: SanicASGITestClient, user_headers, create_secret) -> None: + expiration_timestamp = (datetime.now() + timedelta(seconds=(120 + 15))).isoformat() + secret_1 = await create_secret("secret-1", "value-1", expiration_timestamp=expiration_timestamp) + secret_2 = await create_secret("secret-2", "value-2", expiration_timestamp="2029-12-31") - _, response = await sanic_client.get(f"/api/data/user/secrets/{secret_id}", headers=user_headers) + _, response = await sanic_client.get(f"/api/data/user/secrets/{secret_1["id"]}", headers=user_headers) + assert response.status_code == 200, response.text + assert response.json is not None + assert response.json["name"] == "secret-1" + assert response.json["id"] == secret_1["id"] + _, response = await sanic_client.get(f"/api/data/user/secrets/{secret_2["id"]}", headers=user_headers) assert response.status_code == 200, response.text assert response.json is not None assert response.json["name"] == "secret-2" - assert response.json["id"] == secret_id - assert "value" not in response.json + assert response.json["id"] == secret_2["id"] + + time.sleep(20) + + _, response = await sanic_client.get(f"/api/data/user/secrets/{secret_1["id"]}", headers=user_headers) + assert response.status_code == 404 @pytest.mark.asyncio @@ -84,6 +130,22 @@ async def test_get_all_secrets(sanic_client: SanicASGITestClient, user_headers, assert {s["name"] for s in response.json} == {"secret-1", "secret-2", "secret-3"} +@pytest.mark.asyncio +async def test_get_all_secrets_not_expired(sanic_client: SanicASGITestClient, user_headers, create_secret) -> None: + expiration_timestamp = (datetime.now() + timedelta(seconds=10)).isoformat() + await create_secret("secret-1", "value-1", expiration_timestamp=expiration_timestamp) + await create_secret("secret-2", "value-2") + await create_secret("secret-3", "value-3", expiration_timestamp="2029-12-31") + + time.sleep(15) + + _, response = await sanic_client.get("/api/data/user/secrets", headers=user_headers) + assert response.status_code == 200, response.text + assert response.json is not None + assert {s["name"] for s in response.json} == {"secret-2", "secret-3"} + assert {s["expiration_timestamp"] for s in response.json if s["name"] == "secret-3"} == {"2029-12-31T00:00:00Z"} + + @pytest.mark.asyncio async def test_get_all_secrets_filtered_by_kind(sanic_client, user_headers, create_secret) -> None: await create_secret("secret-1", "value-1") @@ -114,14 +176,10 @@ async def test_get_delete_a_secret(sanic_client: SanicASGITestClient, user_heade secret = await create_secret("secret-2", "value-2") await create_secret("secret-3", "value-3") - secret_id = secret["id"] - - _, response = await sanic_client.delete(f"/api/data/user/secrets/{secret_id}", headers=user_headers) - + _, response = await sanic_client.delete(f"/api/data/user/secrets/{secret["id"]}", headers=user_headers) assert response.status_code == 204, response.text _, response = await sanic_client.get("/api/data/user/secrets", headers=user_headers) - assert response.status_code == 200, response.text assert response.json is not None assert {s["name"] for s in response.json} == {"secret-1", "secret-3"} @@ -133,18 +191,42 @@ async def test_get_update_a_secret(sanic_client: SanicASGITestClient, user_heade secret = await create_secret("secret-2", "value-2") await create_secret("secret-3", "value-3") - secret_id = secret["id"] - payload = {"value": "new-value"} + _, response = await sanic_client.patch( + f"/api/data/user/secrets/{secret["id"]}", headers=user_headers, json={"name": "new-name", "value": "new-value"} + ) + assert response.status_code == 422 - _, response = await sanic_client.patch(f"/api/data/user/secrets/{secret_id}", headers=user_headers, json=payload) + _, response = await sanic_client.patch( + f"/api/data/user/secrets/{secret["id"]}", headers=user_headers, json={"value": "new-value"} + ) + assert response.status_code == 200, response.text + assert response.json is not None + assert response.json["id"] == secret["id"] + assert response.json["name"] == secret["name"] + assert response.json["expiration_timestamp"] is None + assert "value" not in response.json + _, response = await sanic_client.get(f"/api/data/user/secrets/{secret["id"]}", headers=user_headers) assert response.status_code == 200, response.text + assert response.json is not None + assert response.json["id"] == secret["id"] + assert response.json["name"] == secret["name"] + assert response.json["expiration_timestamp"] is None + assert "value" not in response.json - _, response = await sanic_client.get(f"/api/data/user/secrets/{secret_id}", headers=user_headers) + _, response = await sanic_client.patch( + f"/api/data/user/secrets/{secret["id"]}", + headers=user_headers, + json={"value": "newest-value", "expiration_timestamp": "2029-12-31"}, + ) + assert response.status_code == 200, response.text + _, response = await sanic_client.get(f"/api/data/user/secrets/{secret["id"]}", headers=user_headers) assert response.status_code == 200, response.text assert response.json is not None - assert response.json["id"] == secret_id + assert response.json["id"] == secret["id"] + assert response.json["name"] == secret["name"] + assert response.json["expiration_timestamp"] == "2029-12-31T00:00:00Z" assert "value" not in response.json @@ -156,15 +238,11 @@ async def test_cannot_get_another_user_secret( secret = await create_secret("secret-2", "value-2") await create_secret("secret-3", "value-3") - secret_id = secret["id"] - - _, response = await sanic_client.get(f"/api/data/user/secrets/{secret_id}", headers=admin_headers) - + _, response = await sanic_client.get(f"/api/data/user/secrets/{secret["id"]}", headers=admin_headers) assert response.status_code == 404, response.text assert "cannot be found" in response.json["error"]["message"] _, response = await sanic_client.get("/api/data/user/secrets", headers=admin_headers) - assert response.status_code == 200, response.text assert response.json == [] diff --git a/test/bases/renku_data_services/data_api/test_storage_v2.py b/test/bases/renku_data_services/data_api/test_storage_v2.py new file mode 100644 index 000000000..e69de29bb diff --git a/test/conftest.py b/test/conftest.py index fffa7941e..788c993eb 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -252,3 +252,15 @@ def only(iterable, default=None, too_long=None): raise too_long or ValueError(msg) return first_value + + +@pytest.hookimpl(tryfirst=True) +def pytest_runtest_setup(item): + mark = item.get_closest_marker(name="myskip") + if mark: + condition = next(iter(mark.args), True) + reason = mark.kwargs.get("reason") + item.add_marker( + pytest.mark.skipif(not os.getenv("PYTEST_FORCE_RUN_MYSKIPS", False) and condition, reason=reason), + append=False, + ) From cd7b91505d0bc923ca001d513d4f9d2409f4c822 Mon Sep 17 00:00:00 2001 From: olloz26 Date: Thu, 5 Sep 2024 16:22:39 +0200 Subject: [PATCH 07/12] feat: implement review points --- .../renku_data_services/storage/core.py | 25 +++++++++++++++++++ .../renku_data_services/storage/rclone.py | 22 +++++++++++++++- components/renku_data_services/utils/core.py | 9 +++++-- 3 files changed, 53 insertions(+), 3 deletions(-) create mode 100644 components/renku_data_services/storage/core.py diff --git a/components/renku_data_services/storage/core.py b/components/renku_data_services/storage/core.py new file mode 100644 index 000000000..b34fac4f9 --- /dev/null +++ b/components/renku_data_services/storage/core.py @@ -0,0 +1,25 @@ +"""Business logic for storage.""" + +from datetime import datetime + +from renku_data_services import errors +from renku_data_services.storage import models +from renku_data_services.utils.core import get_openbis_pat + + +async def storage_secrets_preparation( + secrets: list[models.CloudStorageSecretUpsert], + storage: models.CloudStorage, + expiration_timestamp: datetime | None = None, +) -> tuple[list[models.CloudStorageSecretUpsert], datetime | None]: + """Prepare the validated secrets so that they can be stored (long-term).""" + if storage.storage_type == "openbis": + try: + ( + secrets[0].value, + expiration_timestamp, + ) = await get_openbis_pat(storage.configuration["host"], secrets[0].value) + except Exception as e: + raise errors.ProgrammingError(message=str(e)) from e + + return secrets, expiration_timestamp diff --git a/components/renku_data_services/storage/rclone.py b/components/renku_data_services/storage/rclone.py index 2a5729f8a..1473968cd 100644 --- a/components/renku_data_services/storage/rclone.py +++ b/components/renku_data_services/storage/rclone.py @@ -142,6 +142,13 @@ def __patch_schema_add_openbis_type(spec: list[dict[str, Any]]) -> None: "Provider": "", "Default": "", "Value": None, + "Examples": [ + { + "Value": "openbis-eln-lims.ethz.ch", + "Help": "Public openBIS demo instance", + "Provider": "", + }, + ], "ShortOpt": "", "Hide": 0, "Required": True, @@ -341,6 +348,19 @@ def validate(self, configuration: Union["RCloneConfig", dict[str, Any]], keep_se provider.validate_config(configuration, keep_sensitive=keep_sensitive) + def validate_sensitive_data( + self, configuration: Union["RCloneConfig", dict[str, Any]], sensitive_data: dict[str, str] + ) -> None: + """Validates whether the provided sensitive data is marked as sensitive in the rclone schema.""" + sensitive_options = self.get_provider(configuration).sensitive_options + sensitive_options_name_lookup = [o.name for o in sensitive_options] + sensitive_data_counter = 0 + for key, value in sensitive_data.items(): + if len(value) > 0 and key in sensitive_options_name_lookup: + sensitive_data_counter += 1 + continue + raise errors.ValidationError(message=f"The '{key}' property is not marked as sensitive.") + async def test_connection( self, configuration: Union["RCloneConfig", dict[str, Any]], source_path: str ) -> ConnectionResult: @@ -572,7 +592,7 @@ class RCloneProviderSchema(BaseModel): @property def required_options(self) -> list[RCloneOption]: """Returns all required options for this provider.""" - return [o for o in self.options if o.required and not o.sensitive] + return [o for o in self.options if o.required] @property def sensitive_options(self) -> list[RCloneOption]: diff --git a/components/renku_data_services/utils/core.py b/components/renku_data_services/utils/core.py index 536a23eb9..e684f4c53 100644 --- a/components/renku_data_services/utils/core.py +++ b/components/renku_data_services/utils/core.py @@ -107,8 +107,13 @@ async def get_openbis_session_token( login = {"method": "login", "params": [username, password], "id": "2", "jsonrpc": "2.0"} async with httpx.AsyncClient(verify=get_ssl_context()) as client: response = await client.post(_get_url(host), json=login, timeout=timeout) - json: dict[str, str] = response.json() - return json["result"] + if response.status_code == 200: + json: dict[str, str] = response.json() + if "result" in json: + return json["result"] + raise Exception("No session token was returned. Username and password may be incorrect.") + + raise Exception("An openBIS session token related request failed.") async def get_openbis_pat( From d731a37710cf5f530d491ed175142b52db819ae1 Mon Sep 17 00:00:00 2001 From: olloz26 Date: Thu, 5 Sep 2024 16:41:59 +0200 Subject: [PATCH 08/12] feat: add openBIS test connection --- .../renku_data_services/storage/rclone.py | 14 +++++++- .../data_api/test_storage.py | 36 ++++++++++++++++++- 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/components/renku_data_services/storage/rclone.py b/components/renku_data_services/storage/rclone.py index 1473968cd..bba651ca2 100644 --- a/components/renku_data_services/storage/rclone.py +++ b/components/renku_data_services/storage/rclone.py @@ -361,6 +361,16 @@ def validate_sensitive_data( continue raise errors.ValidationError(message=f"The '{key}' property is not marked as sensitive.") + def get_real_config(self, configuration: Union["RCloneConfig", dict[str, Any]]) -> dict[str, Any]: + """Converts a Renku rclone configuration to a real rclone config.""" + real_config = dict(configuration) + if configuration["type"] == "openbis": + real_config["type"] = "sftp" + real_config["port"] = "2222" + real_config["user"] = "?" + real_config["pass"] = real_config.pop("session_token") + return real_config + async def test_connection( self, configuration: Union["RCloneConfig", dict[str, Any]], source_path: str ) -> ConnectionResult: @@ -371,7 +381,7 @@ async def test_connection( return ConnectionResult(False, str(e)) # Obscure configuration and transform if needed - obscured_config = await self.obscure_config(configuration) + obscured_config = await self.obscure_config(self.get_real_config(configuration)) transformed_config = self.transform_polybox_switchdriver_config(obscured_config) with tempfile.NamedTemporaryFile(mode="w+", delete=False, encoding="utf-8") as f: @@ -381,6 +391,8 @@ async def test_connection( proc = await asyncio.create_subprocess_exec( "rclone", "lsf", + "--low-level-retries=1", # Connection tests should fail fast. + "--retries=1", # Connection tests should fail fast. "--config", f.name, f"temp:{source_path}", diff --git a/test/bases/renku_data_services/data_api/test_storage.py b/test/bases/renku_data_services/data_api/test_storage.py index 219284a76..5cf89ef59 100644 --- a/test/bases/renku_data_services/data_api/test_storage.py +++ b/test/bases/renku_data_services/data_api/test_storage.py @@ -11,6 +11,7 @@ from renku_data_services.data_api.app import register_all_handlers from renku_data_services.migrations.core import run_migrations_for_app from renku_data_services.storage.rclone import RCloneValidator +from renku_data_services.utils.core import get_openbis_session_token from test.utils import SanicReusableASGITestClient _valid_storage: dict[str, Any] = { @@ -538,7 +539,7 @@ async def test_storage_validate_connection(storage_test_client) -> None: _, res = await storage_test_client.post("/api/data/storage_schema/test_connection", data=json.dumps(body)) assert res.status_code == 422 - body = {"configuration": {"type": "s3", "provider": "AWS"}, "source_path": "doesntexistatall/"} + body = {"configuration": {"type": "s3", "provider": "AWS"}, "source_path": "does_not_exist_at_all/"} _, res = await storage_test_client.post("/api/data/storage_schema/test_connection", data=json.dumps(body)) assert res.status_code == 422 @@ -547,6 +548,39 @@ async def test_storage_validate_connection(storage_test_client) -> None: assert res.status_code == 204 +@pytest.mark.myskip(1 == 1, reason="Depends on a remote openBIS host which may not always be available.") +@pytest.mark.asyncio +async def test_openbis_storage_validate_connection(storage_test_client) -> None: + openbis_session_token = await get_openbis_session_token( + host="openbis-eln-lims.ethz.ch", # Public openBIS demo instance. + username="observer", + password="1234", + ) + storage_test_client, _ = storage_test_client + + body = { + "configuration": { + "type": "openbis", + "host": "openbis-eln-lims.ethz.ch", + "session_token": openbis_session_token, + }, + "source_path": "does_not_exist_at_all/", + } + _, res = await storage_test_client.post("/api/data/storage_schema/test_connection", data=json.dumps(body)) + assert res.status_code == 422 + + body = { + "configuration": { + "type": "openbis", + "host": "openbis-eln-lims.ethz.ch", + "session_token": openbis_session_token, + }, + "source_path": "/", + } + _, res = await storage_test_client.post("/api/data/storage_schema/test_connection", data=json.dumps(body)) + assert res.status_code == 204 + + @pytest.mark.asyncio async def test_storage_validate_error(storage_test_client) -> None: storage_test_client, _ = storage_test_client From f0db04098363191ed4b870adb8015f816c2e3be7 Mon Sep 17 00:00:00 2001 From: olloz26 Date: Thu, 28 Nov 2024 10:38:06 +0100 Subject: [PATCH 09/12] fix: correct a rebase merge --- DEVELOPING.md | 2 +- .../data_connectors/blueprints.py | 51 ++++++++++++++- .../renku_data_services/data_connectors/db.py | 12 +++- ...441beb_add_secret_expiration_timestamp.py} | 12 ++-- .../renku_data_services/storage/core.py | 25 -------- components/renku_data_services/utils/core.py | 4 +- .../renku_data_services/data_api/conftest.py | 33 ++++++++++ .../data_api/test_data_connectors.py | 64 +++++++++++++++++-- .../data_api/test_storage_v2.py | 0 9 files changed, 159 insertions(+), 44 deletions(-) rename components/renku_data_services/migrations/versions/{7bc32829ed2f_add_secret_expiration_timestamp.py => 4d2a21441beb_add_secret_expiration_timestamp.py} (76%) delete mode 100644 components/renku_data_services/storage/core.py delete mode 100644 test/bases/renku_data_services/data_api/test_storage_v2.py diff --git a/DEVELOPING.md b/DEVELOPING.md index 059189296..2cc6a8d70 100644 --- a/DEVELOPING.md +++ b/DEVELOPING.md @@ -117,7 +117,7 @@ function if you prefer to keep your favorite shell. You can run style checks using `make style_checks`. To run the test suite, use `make tests` (you likely need to run in the devcontainer for this to work, as it needs some surrounding services to run). -* Run a specific test e.g.: `poetry run pytest test/bases/renku_data_services/data_api/test_storage_v2.py::test_storage_v2_create_openbis_secret` +* Run a specific test e.g.: `poetry run pytest test/bases/renku_data_services/data_api/test_data_connectors.py::test_create_openbis_data_connector` * Also run tests marked with `@pytest.mark.myskip`: `PYTEST_FORCE_RUN_MYSKIPS=1 make tests` ## Migrations diff --git a/components/renku_data_services/data_connectors/blueprints.py b/components/renku_data_services/data_connectors/blueprints.py index f5469de13..73da71222 100644 --- a/components/renku_data_services/data_connectors/blueprints.py +++ b/components/renku_data_services/data_connectors/blueprints.py @@ -1,6 +1,7 @@ """Data connectors blueprint.""" from dataclasses import dataclass +from datetime import datetime from typing import Any from sanic import Request @@ -8,7 +9,7 @@ from sanic_ext import validate from ulid import ULID -from renku_data_services import base_models +from renku_data_services import base_models, errors from renku_data_services.base_api.auth import ( authenticate, only_authenticated, @@ -31,6 +32,7 @@ DataConnectorSecretRepository, ) from renku_data_services.storage.rclone import RCloneValidator +from renku_data_services.utils.core import get_openbis_pat @dataclass(kw_only=True) @@ -310,10 +312,55 @@ async def _patch_secrets( user: base_models.APIUser, data_connector_id: ULID, body: apispec.DataConnectorSecretPatchList, + validator: RCloneValidator, ) -> JSONResponse: unsaved_secrets = validate_data_connector_secrets_patch(put=body) + data_connector = await self.data_connector_repo.get_data_connector( + user=user, data_connector_id=data_connector_id + ) + storage = data_connector.storage + provider = validator.providers[storage.storage_type] + sensitive_lookup = [o.name for o in provider.options if o.sensitive] + for secret in unsaved_secrets: + if secret.name in sensitive_lookup: + continue + raise errors.ValidationError( + message=f"The '{secret.name}' property is not marked sensitive and can not be saved in the secret " + f"storage." + ) + expiration_timestamp = None + + if storage.storage_type == "openbis": + + async def openbis_transform_session_token_to_pat() -> ( + tuple[list[models.DataConnectorSecretUpdate], datetime] + ): + if len(unsaved_secrets) == 1 and unsaved_secrets[0].name == "session_token": + if unsaved_secrets[0].value is not None: + try: + openbis_pat = await get_openbis_pat( + storage.configuration["host"], unsaved_secrets[0].value + ) + return ( + [models.DataConnectorSecretUpdate(name="session_token", value=openbis_pat[0])], + openbis_pat[1], + ) + except Exception as e: + raise errors.ProgrammingError(message=str(e)) + raise errors.ValidationError(message="The openBIS session token must be a string value.") + + raise errors.ValidationError(message="The openBIS storage has only one secret: session_token") + + ( + unsaved_secrets, + expiration_timestamp, + ) = await openbis_transform_session_token_to_pat() + secrets = await self.data_connector_secret_repo.patch_data_connector_secrets( - user=user, data_connector_id=data_connector_id, secrets=unsaved_secrets + user=user, + data_connector_id=data_connector_id, + secrets=unsaved_secrets, + expiration_timestamp=expiration_timestamp, ) return validated_json( apispec.DataConnectorSecretsList, [self._dump_data_connector_secret(secret) for secret in secrets] diff --git a/components/renku_data_services/data_connectors/db.py b/components/renku_data_services/data_connectors/db.py index 6bda29641..4b6995296 100644 --- a/components/renku_data_services/data_connectors/db.py +++ b/components/renku_data_services/data_connectors/db.py @@ -1,6 +1,7 @@ """Adapters for data connectors database classes.""" from collections.abc import AsyncIterator, Callable +from datetime import datetime from typing import TypeVar from cryptography.hazmat.primitives.asymmetric import rsa @@ -554,7 +555,11 @@ async def get_data_connector_secrets( return [secret.dump() for secret in secrets] async def patch_data_connector_secrets( - self, user: base_models.APIUser, data_connector_id: ULID, secrets: list[models.DataConnectorSecretUpdate] + self, + user: base_models.APIUser, + data_connector_id: ULID, + secrets: list[models.DataConnectorSecretUpdate], + expiration_timestamp: datetime | None, ) -> list[models.DataConnectorSecret]: """Create, update or remove data connector secrets.""" if user.id is None: @@ -598,7 +603,9 @@ async def patch_data_connector_secrets( if data_connector_secret_orm := existing_secrets_as_dict.get(name): data_connector_secret_orm.secret.update( - encrypted_value=encrypted_value, encrypted_key=encrypted_key + encrypted_value=encrypted_value, + encrypted_key=encrypted_key, + expiration_timestamp=expiration_timestamp, ) else: secret_orm = secrets_schemas.SecretORM( @@ -607,6 +614,7 @@ async def patch_data_connector_secrets( encrypted_value=encrypted_value, encrypted_key=encrypted_key, kind=SecretKind.storage, + expiration_timestamp=expiration_timestamp, ) data_connector_secret_orm = schemas.DataConnectorSecretORM( name=name, diff --git a/components/renku_data_services/migrations/versions/7bc32829ed2f_add_secret_expiration_timestamp.py b/components/renku_data_services/migrations/versions/4d2a21441beb_add_secret_expiration_timestamp.py similarity index 76% rename from components/renku_data_services/migrations/versions/7bc32829ed2f_add_secret_expiration_timestamp.py rename to components/renku_data_services/migrations/versions/4d2a21441beb_add_secret_expiration_timestamp.py index 0812b2adb..d7f4b22db 100644 --- a/components/renku_data_services/migrations/versions/7bc32829ed2f_add_secret_expiration_timestamp.py +++ b/components/renku_data_services/migrations/versions/4d2a21441beb_add_secret_expiration_timestamp.py @@ -1,8 +1,8 @@ -"""add_secret_expiration_timestamp +"""add secret expiration timestamp -Revision ID: 7bc32829ed2f -Revises: 9058bf0a1a12 -Create Date: 2024-08-21 12:38:30.932694 +Revision ID: 4d2a21441beb +Revises: 1ef98b967767 +Create Date: 2024-11-21 17:01:56.468831 """ @@ -10,8 +10,8 @@ from alembic import op # revision identifiers, used by Alembic. -revision = "7bc32829ed2f" -down_revision = "9058bf0a1a12" +revision = "4d2a21441beb" +down_revision = "1ef98b967767" branch_labels = None depends_on = None diff --git a/components/renku_data_services/storage/core.py b/components/renku_data_services/storage/core.py deleted file mode 100644 index b34fac4f9..000000000 --- a/components/renku_data_services/storage/core.py +++ /dev/null @@ -1,25 +0,0 @@ -"""Business logic for storage.""" - -from datetime import datetime - -from renku_data_services import errors -from renku_data_services.storage import models -from renku_data_services.utils.core import get_openbis_pat - - -async def storage_secrets_preparation( - secrets: list[models.CloudStorageSecretUpsert], - storage: models.CloudStorage, - expiration_timestamp: datetime | None = None, -) -> tuple[list[models.CloudStorageSecretUpsert], datetime | None]: - """Prepare the validated secrets so that they can be stored (long-term).""" - if storage.storage_type == "openbis": - try: - ( - secrets[0].value, - expiration_timestamp, - ) = await get_openbis_pat(storage.configuration["host"], secrets[0].value) - except Exception as e: - raise errors.ProgrammingError(message=str(e)) from e - - return secrets, expiration_timestamp diff --git a/components/renku_data_services/utils/core.py b/components/renku_data_services/utils/core.py index e684f4c53..587f5528d 100644 --- a/components/renku_data_services/utils/core.py +++ b/components/renku_data_services/utils/core.py @@ -105,7 +105,7 @@ async def get_openbis_session_token( ) -> str: """Requests an openBIS session token with the user's login credentials.""" login = {"method": "login", "params": [username, password], "id": "2", "jsonrpc": "2.0"} - async with httpx.AsyncClient(verify=get_ssl_context()) as client: + async with httpx.AsyncClient(verify=get_ssl_context(), timeout=5) as client: response = await client.post(_get_url(host), json=login, timeout=timeout) if response.status_code == 200: json: dict[str, str] = response.json() @@ -126,7 +126,7 @@ async def get_openbis_pat( """Requests an openBIS PAT with an openBIS session ID.""" url = _get_url(host) - async with httpx.AsyncClient(verify=get_ssl_context()) as client: + async with httpx.AsyncClient(verify=get_ssl_context(), timeout=5) as client: get_server_information = {"method": "getServerInformation", "params": [session_id], "id": "2", "jsonrpc": "2.0"} response = await client.post(url, json=get_server_information, timeout=timeout) if response.status_code == 200: diff --git a/test/bases/renku_data_services/data_api/conftest.py b/test/bases/renku_data_services/data_api/conftest.py index 9259ab07a..9b1514282 100644 --- a/test/bases/renku_data_services/data_api/conftest.py +++ b/test/bases/renku_data_services/data_api/conftest.py @@ -371,6 +371,39 @@ async def create_data_connector_helper( return create_data_connector_helper +@pytest_asyncio.fixture +def create_openbis_data_connector(sanic_client: SanicASGITestClient, regular_user: UserInfo, user_headers): + async def create_openbis_data_connector_helper( + name: str, session_token: str, user: UserInfo | None = None, headers: dict[str, str] | None = None, **payload + ) -> Any: + user = user or regular_user + headers = headers or user_headers + dc_payload = { + "name": name, + "description": "An openBIS data connector", + "visibility": "private", + "namespace": user.namespace.slug, + "storage": { + "configuration": { + "type": "openbis", + "host": "openbis-eln-lims.ethz.ch", # Public openBIS demo instance. + "session_token": session_token, + }, + "source_path": "/", + "target_path": "my/target", + }, + "keywords": ["keyword 1", "keyword.2", "keyword-3", "KEYWORD_4"], + } + dc_payload.update(payload) + + _, response = await sanic_client.post("/api/data/data_connectors", headers=headers, json=dc_payload) + + assert response.status_code == 201, response.text + return response.json + + return create_openbis_data_connector_helper + + @pytest_asyncio.fixture async def create_data_connector_and_link_project( sanic_client, regular_user, user_headers, admin_user, admin_headers, create_data_connector diff --git a/test/bases/renku_data_services/data_api/test_data_connectors.py b/test/bases/renku_data_services/data_api/test_data_connectors.py index 457fa472d..1ca8515b0 100644 --- a/test/bases/renku_data_services/data_api/test_data_connectors.py +++ b/test/bases/renku_data_services/data_api/test_data_connectors.py @@ -2,9 +2,9 @@ from sanic_testing.testing import SanicASGITestClient from renku_data_services.users.models import UserInfo +from renku_data_services.utils.core import get_openbis_session_token from test.bases.renku_data_services.data_api.utils import merge_headers - @pytest.mark.asyncio async def test_post_data_connector(sanic_client: SanicASGITestClient, regular_user: UserInfo, user_headers) -> None: payload = { @@ -1073,6 +1073,14 @@ async def test_patch_data_connector_secrets( assert len(secrets) == 2 assert {s["name"] for s in secrets} == {"access_key_id", "secret_access_key"} + payload = [ + {"name": "not_sensitive", "value": "not_sensitive_value"}, + ] + _, response = await sanic_client.patch( + f"/api/data/data_connectors/{data_connector_id}/secrets", headers=user_headers, json=payload + ) + assert response.status_code == 422, response.json + @pytest.mark.asyncio async def test_patch_data_connector_secrets_update_secrets( @@ -1142,7 +1150,7 @@ async def test_patch_data_connector_secrets_add_and_remove_secrets( payload = [ {"name": "access_key_id", "value": "new access key id value"}, {"name": "secret_access_key", "value": None}, - {"name": "password", "value": "password"}, + {"name": "sse_kms_key_id", "value": "password"}, ] _, response = await sanic_client.patch( f"/api/data/data_connectors/{data_connector_id}/secrets", headers=user_headers, json=payload @@ -1152,7 +1160,7 @@ async def test_patch_data_connector_secrets_add_and_remove_secrets( assert response.json is not None secrets = response.json assert len(secrets) == 2 - assert {s["name"] for s in secrets} == {"access_key_id", "password"} + assert {s["name"] for s in secrets} == {"access_key_id", "sse_kms_key_id"} new_access_key_id_secret_id = next(filter(lambda s: s["name"] == "access_key_id", secrets), None) assert new_access_key_id_secret_id == access_key_id_secret_id @@ -1162,15 +1170,14 @@ async def test_patch_data_connector_secrets_add_and_remove_secrets( assert response.json is not None secrets = response.json assert len(secrets) == 2 - assert {s["name"] for s in secrets} == {"access_key_id", "password"} + assert {s["name"] for s in secrets} == {"access_key_id", "sse_kms_key_id"} # Check the associated secrets _, response = await sanic_client.get("/api/data/user/secrets", params={"kind": "storage"}, headers=user_headers) - assert response.status_code == 200 assert response.json is not None assert len(response.json) == 2 - assert {s["name"] for s in secrets} == {"access_key_id", "password"} + assert {s["name"] for s in secrets} == {"access_key_id", "sse_kms_key_id"} @pytest.mark.asyncio @@ -1210,6 +1217,51 @@ async def test_delete_data_connector_secrets( assert response.json == [], response.json +@pytest.mark.myskip(1 == 1, reason="Depends on a remote openBIS host which may not always be available.") +@pytest.mark.asyncio +async def test_create_openbis_data_connector(sanic_client, create_openbis_data_connector, user_headers) -> None: + openbis_session_token = await get_openbis_session_token( + host="openbis-eln-lims.ethz.ch", # Public openBIS demo instance. + username="observer", + password="1234", + ) + data_connector = await create_openbis_data_connector( + "openBIS data connector 1", session_token=openbis_session_token + ) + data_connector_id = data_connector["id"] + + payload = [ + {"name": "session_token", "value": openbis_session_token}, + ] + _, response = await sanic_client.patch( + f"/api/data/data_connectors/{data_connector_id}/secrets", headers=user_headers, json=payload + ) + assert response.status_code == 200, response.json + assert {s["name"] for s in response.json} == {"session_token"} + created_secret_ids = {s["secret_id"] for s in response.json} + assert len(created_secret_ids) == 1 + assert response.json[0].keys() == {"secret_id", "name"} + + +@pytest.mark.myskip(1 == 1, reason="Depends on a remote openBIS host which may not always be available.") +@pytest.mark.asyncio +async def test_create_openbis_data_connector_with_invalid_session_token( + sanic_client, create_openbis_data_connector, user_headers +) -> None: + invalid_openbis_session_token = "1234" + data_connector = await create_openbis_data_connector("openBIS data connector 1", invalid_openbis_session_token) + data_connector_id = data_connector["id"] + + payload = [ + {"name": "session_token", "value": invalid_openbis_session_token}, + ] + _, response = await sanic_client.patch( + f"/api/data/data_connectors/{data_connector_id}/secrets", headers=user_headers, json=payload + ) + assert response.status_code == 500, response.json + assert response.json["error"]["message"] == "An openBIS personal access token related request failed." + + @pytest.mark.asyncio async def test_get_project_permissions_unauthorized( sanic_client, create_data_connector, admin_headers, admin_user, user_headers diff --git a/test/bases/renku_data_services/data_api/test_storage_v2.py b/test/bases/renku_data_services/data_api/test_storage_v2.py deleted file mode 100644 index e69de29bb..000000000 From 116fd28873a1488b67665302f6703204460fe559 Mon Sep 17 00:00:00 2001 From: olloz26 Date: Thu, 28 Nov 2024 11:38:04 +0100 Subject: [PATCH 10/12] fix: correct a rebase merge --- ...=> 57facc53ae84_add_secret_expiration_timestamp.py} | 10 +++++----- .../data_api/test_data_connectors.py | 1 + 2 files changed, 6 insertions(+), 5 deletions(-) rename components/renku_data_services/migrations/versions/{4d2a21441beb_add_secret_expiration_timestamp.py => 57facc53ae84_add_secret_expiration_timestamp.py} (81%) diff --git a/components/renku_data_services/migrations/versions/4d2a21441beb_add_secret_expiration_timestamp.py b/components/renku_data_services/migrations/versions/57facc53ae84_add_secret_expiration_timestamp.py similarity index 81% rename from components/renku_data_services/migrations/versions/4d2a21441beb_add_secret_expiration_timestamp.py rename to components/renku_data_services/migrations/versions/57facc53ae84_add_secret_expiration_timestamp.py index d7f4b22db..ed88aba4f 100644 --- a/components/renku_data_services/migrations/versions/4d2a21441beb_add_secret_expiration_timestamp.py +++ b/components/renku_data_services/migrations/versions/57facc53ae84_add_secret_expiration_timestamp.py @@ -1,8 +1,8 @@ """add secret expiration timestamp -Revision ID: 4d2a21441beb -Revises: 1ef98b967767 -Create Date: 2024-11-21 17:01:56.468831 +Revision ID: 57facc53ae84 +Revises: 08ac2714e8e2 +Create Date: 2024-11-28 10:31:05.683682 """ @@ -10,8 +10,8 @@ from alembic import op # revision identifiers, used by Alembic. -revision = "4d2a21441beb" -down_revision = "1ef98b967767" +revision = "57facc53ae84" +down_revision = "08ac2714e8e2" branch_labels = None depends_on = None diff --git a/test/bases/renku_data_services/data_api/test_data_connectors.py b/test/bases/renku_data_services/data_api/test_data_connectors.py index 1ca8515b0..910702483 100644 --- a/test/bases/renku_data_services/data_api/test_data_connectors.py +++ b/test/bases/renku_data_services/data_api/test_data_connectors.py @@ -5,6 +5,7 @@ from renku_data_services.utils.core import get_openbis_session_token from test.bases.renku_data_services.data_api.utils import merge_headers + @pytest.mark.asyncio async def test_post_data_connector(sanic_client: SanicASGITestClient, regular_user: UserInfo, user_headers) -> None: payload = { From e2aa85169fe3299ce38c3f20c481bc76aa56d875 Mon Sep 17 00:00:00 2001 From: olloz26 Date: Wed, 4 Dec 2024 11:26:10 +0100 Subject: [PATCH 11/12] feat: convert openBIS cloud storage configurations into valid rclone configurations before starting a session --- .../notebooks/api/schemas/cloud_storage.py | 36 +++++++++++-------- .../renku_data_services/storage/rclone.py | 11 ++++-- 2 files changed, 29 insertions(+), 18 deletions(-) diff --git a/components/renku_data_services/notebooks/api/schemas/cloud_storage.py b/components/renku_data_services/notebooks/api/schemas/cloud_storage.py index f4c561442..58662e090 100644 --- a/components/renku_data_services/notebooks/api/schemas/cloud_storage.py +++ b/components/renku_data_services/notebooks/api/schemas/cloud_storage.py @@ -210,40 +210,46 @@ def get_manifest_patch( return patches def config_string(self, name: str) -> str: - """Convert configuration oblect to string representation. + """Convert configuration object to string representation. Needed to create RClone compatible INI files. """ if not self.configuration: raise ValidationError("Missing configuration for cloud storage") - - # Transform configuration for polybox or switchDrive + # TODO Use RCloneValidator.get_real_configuration(...) instead. + real_config = dict(self.configuration) + # Transform configuration for polybox, switchDrive or openBIS storage_type = self.configuration.get("type", "") access = self.configuration.get("provider", "") if storage_type == "polybox" or storage_type == "switchDrive": - self.configuration["type"] = "webdav" - self.configuration["provider"] = "" + real_config["type"] = "webdav" + real_config["provider"] = "" + elif storage_type == "s3" and access == "Switch": + # Switch is a fake provider we add for users, we need to replace it since rclone itself + # doesn't know it + real_config["provider"] = "Other" + elif storage_type == "openbis": + real_config["type"] = "sftp" + real_config["port"] = "2222" + real_config["user"] = "?" + real_config["pass"] = real_config.pop("session_token") if access == "shared" and storage_type == "polybox": - self.configuration["url"] = "https://polybox.ethz.ch/public.php/webdav/" + real_config["url"] = "https://polybox.ethz.ch/public.php/webdav/" elif access == "shared" and storage_type == "switchDrive": - self.configuration["url"] = "https://drive.switch.ch/public.php/webdav/" + real_config["url"] = "https://drive.switch.ch/public.php/webdav/" elif access == "personal" and storage_type == "polybox": - self.configuration["url"] = "https://polybox.ethz.ch/remote.php/webdav/" + real_config["url"] = "https://polybox.ethz.ch/remote.php/webdav/" elif access == "personal" and storage_type == "switchDrive": - self.configuration["url"] = "https://drive.switch.ch/remote.php/webdav/" + real_config["url"] = "https://drive.switch.ch/remote.php/webdav/" # Extract the user from the public link if access == "shared" and storage_type in {"polybox", "switchDrive"}: public_link = self.configuration.get("public_link", "") user_identifier = public_link.split("/")[-1] - self.configuration["user"] = user_identifier + real_config["user"] = user_identifier - if self.configuration["type"] == "s3" and self.configuration.get("provider", None) == "Switch": - # Switch is a fake provider we add for users, we need to replace it since rclone itself - # doesn't know it - self.configuration["provider"] = "Other" parser = ConfigParser() parser.add_section(name) @@ -252,7 +258,7 @@ def _stringify(value: Any) -> str: return "true" if value else "false" return str(value) - for k, v in self.configuration.items(): + for k, v in real_config.items(): parser.set(name, k, _stringify(v)) stringio = StringIO() parser.write(stringio) diff --git a/components/renku_data_services/storage/rclone.py b/components/renku_data_services/storage/rclone.py index bba651ca2..42b8e3fe0 100644 --- a/components/renku_data_services/storage/rclone.py +++ b/components/renku_data_services/storage/rclone.py @@ -361,10 +361,15 @@ def validate_sensitive_data( continue raise errors.ValidationError(message=f"The '{key}' property is not marked as sensitive.") - def get_real_config(self, configuration: Union["RCloneConfig", dict[str, Any]]) -> dict[str, Any]: + def get_real_configuration(self, configuration: Union["RCloneConfig", dict[str, Any]]) -> dict[str, Any]: """Converts a Renku rclone configuration to a real rclone config.""" real_config = dict(configuration) - if configuration["type"] == "openbis": + + if real_config["type"] == "s3" and real_config.get("provider") == "Switch": + # Switch is a fake provider we add for users, we need to replace it since rclone itself + # doesn't know it + real_config["provider"] = "Other" + elif configuration["type"] == "openbis": real_config["type"] = "sftp" real_config["port"] = "2222" real_config["user"] = "?" @@ -381,7 +386,7 @@ async def test_connection( return ConnectionResult(False, str(e)) # Obscure configuration and transform if needed - obscured_config = await self.obscure_config(self.get_real_config(configuration)) + obscured_config = await self.obscure_config(self.get_real_configuration(configuration)) transformed_config = self.transform_polybox_switchdriver_config(obscured_config) with tempfile.NamedTemporaryFile(mode="w+", delete=False, encoding="utf-8") as f: From 473e5ea0ab9fdc1e32b44e86a192e09beda1e664 Mon Sep 17 00:00:00 2001 From: olloz26 Date: Thu, 12 Dec 2024 11:38:08 +0100 Subject: [PATCH 12/12] feat: convert openBIS cloud storage configurations into valid rclone configurations before starting a session (next try) --- .../notebooks/api/schemas/cloud_storage.py | 27 +++++++++---------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/components/renku_data_services/notebooks/api/schemas/cloud_storage.py b/components/renku_data_services/notebooks/api/schemas/cloud_storage.py index 58662e090..865cc0ae7 100644 --- a/components/renku_data_services/notebooks/api/schemas/cloud_storage.py +++ b/components/renku_data_services/notebooks/api/schemas/cloud_storage.py @@ -217,38 +217,37 @@ def config_string(self, name: str) -> str: if not self.configuration: raise ValidationError("Missing configuration for cloud storage") # TODO Use RCloneValidator.get_real_configuration(...) instead. - real_config = dict(self.configuration) # Transform configuration for polybox, switchDrive or openBIS storage_type = self.configuration.get("type", "") access = self.configuration.get("provider", "") if storage_type == "polybox" or storage_type == "switchDrive": - real_config["type"] = "webdav" - real_config["provider"] = "" + self.configuration["type"] = "webdav" + self.configuration["provider"] = "" elif storage_type == "s3" and access == "Switch": # Switch is a fake provider we add for users, we need to replace it since rclone itself # doesn't know it - real_config["provider"] = "Other" + self.configuration["provider"] = "Other" elif storage_type == "openbis": - real_config["type"] = "sftp" - real_config["port"] = "2222" - real_config["user"] = "?" - real_config["pass"] = real_config.pop("session_token") + self.configuration["type"] = "sftp" + self.configuration["port"] = "2222" + self.configuration["user"] = "?" + self.configuration["pass"] = self.configuration.pop("session_token", self.configuration["pass"]) if access == "shared" and storage_type == "polybox": - real_config["url"] = "https://polybox.ethz.ch/public.php/webdav/" + self.configuration["url"] = "https://polybox.ethz.ch/public.php/webdav/" elif access == "shared" and storage_type == "switchDrive": - real_config["url"] = "https://drive.switch.ch/public.php/webdav/" + self.configuration["url"] = "https://drive.switch.ch/public.php/webdav/" elif access == "personal" and storage_type == "polybox": - real_config["url"] = "https://polybox.ethz.ch/remote.php/webdav/" + self.configuration["url"] = "https://polybox.ethz.ch/remote.php/webdav/" elif access == "personal" and storage_type == "switchDrive": - real_config["url"] = "https://drive.switch.ch/remote.php/webdav/" + self.configuration["url"] = "https://drive.switch.ch/remote.php/webdav/" # Extract the user from the public link if access == "shared" and storage_type in {"polybox", "switchDrive"}: public_link = self.configuration.get("public_link", "") user_identifier = public_link.split("/")[-1] - real_config["user"] = user_identifier + self.configuration["user"] = user_identifier parser = ConfigParser() parser.add_section(name) @@ -258,7 +257,7 @@ def _stringify(value: Any) -> str: return "true" if value else "false" return str(value) - for k, v in real_config.items(): + for k, v in self.configuration.items(): parser.set(name, k, _stringify(v)) stringio = StringIO() parser.write(stringio)