From 93c9739513dd043166f74f019840103b3d7fc864 Mon Sep 17 00:00:00 2001 From: olloz26 Date: Tue, 21 Oct 2025 13:58:29 +0200 Subject: [PATCH] feat: add custom openBIS type --- DEVELOPING.md | 2 + .../data_connectors/blueprints.py | 80 ++++++++++- .../renku_data_services/data_connectors/db.py | 12 +- ...a39a3bc_add_secret_expiration_timestamp.py | 30 ++++ .../notebooks/api/schemas/cloud_storage.py | 26 ++-- components/renku_data_services/secrets/db.py | 33 +++-- .../renku_data_services/secrets/models.py | 8 +- components/renku_data_services/secrets/orm.py | 11 +- .../renku_data_services/storage/rclone.py | 32 ++++- .../storage/rclone_patches.py | 61 ++++++++ .../renku_data_services/users/api.spec.yaml | 29 ++-- .../renku_data_services/users/apispec.py | 42 ++++-- .../renku_data_services/users/blueprints.py | 8 +- components/renku_data_services/users/core.py | 6 +- components/renku_data_services/utils/core.py | 80 +++++++++++ .../data_api/__snapshots__/test_storage.ambr | 44 ++++++ .../renku_data_services/data_api/conftest.py | 33 +++++ .../data_api/test_data_connectors.py | 69 ++++++++- .../data_api/test_secret.py | 136 ++++++++++++++---- .../data_api/test_storage.py | 36 ++++- test/conftest.py | 12 ++ 21 files changed, 701 insertions(+), 89 deletions(-) create mode 100644 components/renku_data_services/migrations/versions/aa8f1a39a3bc_add_secret_expiration_timestamp.py diff --git a/DEVELOPING.md b/DEVELOPING.md index 8ea65215e..3fefec3b8 100644 --- a/DEVELOPING.md +++ b/DEVELOPING.md @@ -119,6 +119,8 @@ 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 -v 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` We use [Syrupy](https://github.com/syrupy-project/syrupy) for snapshotting data in tests. diff --git a/components/renku_data_services/data_connectors/blueprints.py b/components/renku_data_services/data_connectors/blueprints.py index 4ea2913cf..d1f5969b3 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, @@ -38,8 +39,8 @@ DataConnectorRepository, DataConnectorSecretRepository, ) -from renku_data_services.errors import errors from renku_data_services.storage.rclone import RCloneValidator +from renku_data_services.utils.core import get_openbis_pat @dataclass(kw_only=True) @@ -418,8 +419,15 @@ async def _get_secrets( secrets = await self.data_connector_secret_repo.get_data_connector_secrets( user=user, data_connector_id=data_connector_id ) + data_connector = await self.data_connector_repo.get_data_connector( + user=user, data_connector_id=data_connector_id + ) return validated_json( - apispec.DataConnectorSecretsList, [self._dump_data_connector_secret(secret) for secret in secrets] + apispec.DataConnectorSecretsList, + [ + self._dump_data_connector_secret(secret) + for secret in self._adjust_secrets(secrets, data_connector.storage) + ], ) return "/data_connectors//secrets", ["GET"], _get_secrets @@ -435,13 +443,59 @@ 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="pass", value=openbis_pat[0])], + openbis_pat[1], + ) + except Exception as e: + raise errors.ProgrammingError(message=str(e)) from 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] + apispec.DataConnectorSecretsList, + [self._dump_data_connector_secret(secret) for secret in self._adjust_secrets(secrets, storage)], ) return "/data_connectors//secrets", ["PATCH"], _patch_secrets @@ -508,6 +562,22 @@ def _dump_data_connector_to_project_link(link: models.DataConnectorToProjectLink created_by=link.created_by, ) + @staticmethod + def _adjust_secrets( + secrets: list[models.DataConnectorSecret], storage: models.CloudStorageCore + ) -> list[models.DataConnectorSecret]: + if storage.storage_type == "openbis": + for i, secret in enumerate(secrets): + if secret.name == "pass": + secrets[i] = models.DataConnectorSecret( + name="session_token", + user_id=secret.user_id, + data_connector_id=secret.data_connector_id, + secret_id=secret.secret_id, + ) + break + return secrets + @staticmethod def _dump_data_connector_secret(secret: models.DataConnectorSecret) -> dict[str, Any]: """Dumps a data connector secret for API responses.""" diff --git a/components/renku_data_services/data_connectors/db.py b/components/renku_data_services/data_connectors/db.py index 8acc05c94..6effa62b3 100644 --- a/components/renku_data_services/data_connectors/db.py +++ b/components/renku_data_services/data_connectors/db.py @@ -4,6 +4,7 @@ import string from collections.abc import AsyncIterator, Callable, Sequence from contextlib import suppress +from datetime import datetime from typing import TypeVar from cryptography.hazmat.primitives.asymmetric import rsa @@ -886,7 +887,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: @@ -935,7 +940,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_name = f"{data_connector.name[:45]} - {name[:45]}" @@ -949,6 +956,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/aa8f1a39a3bc_add_secret_expiration_timestamp.py b/components/renku_data_services/migrations/versions/aa8f1a39a3bc_add_secret_expiration_timestamp.py new file mode 100644 index 000000000..d0f3160ac --- /dev/null +++ b/components/renku_data_services/migrations/versions/aa8f1a39a3bc_add_secret_expiration_timestamp.py @@ -0,0 +1,30 @@ +"""add secret expiration timestamp + +Revision ID: aa8f1a39a3bc +Revises: d437be68a4fb +Create Date: 2025-10-21 13:12:56.882429 + +""" + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision = "aa8f1a39a3bc" +down_revision = "d437be68a4fb" +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/notebooks/api/schemas/cloud_storage.py b/components/renku_data_services/notebooks/api/schemas/cloud_storage.py index a2dbe41f0..01b368e46 100644 --- a/components/renku_data_services/notebooks/api/schemas/cloud_storage.py +++ b/components/renku_data_services/notebooks/api/schemas/cloud_storage.py @@ -228,15 +228,11 @@ def config_string(self, name: str) -> str: if not self.configuration: raise ValidationError("Missing configuration for cloud storage") - # Transform configuration for polybox, switchDrive or sftp + # TODO Use RCloneValidator.get_real_configuration(...) instead. + # Transform configuration for polybox, switchDrive, openbis or sftp storage_type = self.configuration.get("type", "") access = self.configuration.get("provider", "") - if storage_type == "sftp": - # Do not allow retries for sftp - # Reference: https://rclone.org/docs/#globalconfig - self.configuration["override.low_level_retries"] = 1 - if storage_type == "polybox" or storage_type == "switchDrive": self.configuration["type"] = "webdav" self.configuration["provider"] = "" @@ -244,6 +240,20 @@ def config_string(self, name: str) -> str: # time for touched files to be temporarily set to `1999-09-04` which causes the text # editor to complain that the file has changed and whether it should overwrite new changes. self.configuration["vendor"] = "owncloud" + 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 + self.configuration["provider"] = "Other" + elif storage_type == "openbis": + self.configuration["type"] = "sftp" + self.configuration["port"] = "2222" + self.configuration["user"] = "?" + self.configuration["pass"] = self.configuration.pop("session_token", self.configuration["pass"]) + + if storage_type == "sftp" or storage_type == "openbis": + # Do not allow retries for sftp + # Reference: https://rclone.org/docs/#globalconfig + self.configuration["override.low_level_retries"] = 1 if access == "shared" and storage_type == "polybox": self.configuration["url"] = "https://polybox.ethz.ch/public.php/webdav/" @@ -260,10 +270,6 @@ def config_string(self, name: str) -> str: user_identifier = public_link.split("/")[-1] self.configuration["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) diff --git a/components/renku_data_services/secrets/db.py b/components/renku_data_services/secrets/db.py index 1c106e1c4..104459a66 100644 --- a/components/renku_data_services/secrets/db.py +++ b/components/renku_data_services/secrets/db.py @@ -6,12 +6,12 @@ import random import string from collections.abc import AsyncGenerator, Callable, Sequence -from datetime import UTC, datetime +from datetime import UTC, datetime, timedelta from typing import cast from cryptography.hazmat.primitives.asymmetric import rsa from prometheus_client import Counter, Enum -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 @@ -147,11 +147,23 @@ def __init__( self.user_repo = user_repo self.secret_service_public_key = secret_service_public_key + 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] @@ -160,7 +172,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: """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 not orm: @@ -187,11 +199,12 @@ async def insert_secret(self, requested_by: APIUser, secret: UnsavedSecret) -> S async with self.session_maker() as session, session.begin(): secret_orm = SecretORM( name=secret.name, - default_filename=default_filename, user_id=requested_by.id, encrypted_value=encrypted_value, encrypted_key=encrypted_key, kind=secret.kind, + expiration_timestamp=secret.expiration_timestamp, + default_filename=default_filename, ) session.add(secret_orm) @@ -212,9 +225,7 @@ async def update_secret(self, requested_by: APIUser, secret_id: ULID, patch: Sec """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") @@ -239,6 +250,8 @@ async def update_secret(self, requested_by: APIUser, secret_id: ULID, patch: Sec secret_value=patch.secret_value, ) secret.update(encrypted_value=encrypted_value, encrypted_key=encrypted_key) + if patch.expiration_timestamp is not None: + secret.expiration_timestamp = patch.expiration_timestamp return secret.dump() @@ -247,9 +260,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 11f45ffaa..ffd676fc8 100644 --- a/components/renku_data_services/secrets/models.py +++ b/components/renku_data_services/secrets/models.py @@ -8,6 +8,7 @@ from cryptography.hazmat.primitives.asymmetric import rsa from kubernetes import client as k8s_client +from pydantic import Field from ulid import ULID from renku_data_services.app_config import logging @@ -39,6 +40,7 @@ class Secret: encrypted_value: bytes = field(repr=False) encrypted_key: bytes = field(repr=False) kind: SecretKind + expiration_timestamp: datetime | None = Field(default=None) modification_date: datetime session_secret_slot_ids: list[ULID] @@ -115,9 +117,10 @@ class UnsavedSecret: """Model to request the creation of a new user secret.""" name: str - default_filename: str | None secret_value: str = field(repr=False) kind: SecretKind + expiration_timestamp: datetime | None = None + default_filename: str | None @dataclass(frozen=True, eq=True, kw_only=True) @@ -125,5 +128,6 @@ class SecretPatch: """Model for changes requested on a user secret.""" name: str | None - default_filename: str | None secret_value: str | None = field(repr=False) + expiration_timestamp: datetime | None = None + default_filename: str | None diff --git a/components/renku_data_services/secrets/orm.py b/components/renku_data_services/secrets/orm.py index 5bdbcd350..94be073f4 100644 --- a/components/renku_data_services/secrets/orm.py +++ b/components/renku_data_services/secrets/orm.py @@ -48,6 +48,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), @@ -72,17 +75,21 @@ def dump(self) -> models.Secret: return models.Secret( id=self.id, name=self.name, - default_filename=self.default_filename, encrypted_value=self.encrypted_value, encrypted_key=self.encrypted_key, kind=self.kind, + expiration_timestamp=self.expiration_timestamp, modification_date=self.modification_date, + default_filename=self.default_filename, session_secret_slot_ids=[item.secret_slot_id for item in self.session_secrets], data_connector_ids=[item.data_connector_id for item in self.data_connector_secrets], ) - def update(self, encrypted_value: bytes, encrypted_key: bytes) -> None: + def update( + self, encrypted_value: bytes, encrypted_key: bytes, expiration_timestamp: datetime | None = 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) diff --git a/components/renku_data_services/storage/rclone.py b/components/renku_data_services/storage/rclone.py index f9c9a1f2a..8bde8fa50 100644 --- a/components/renku_data_services/storage/rclone.py +++ b/components/renku_data_services/storage/rclone.py @@ -52,6 +52,34 @@ 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.") + + 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 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"] = "?" + 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: @@ -62,7 +90,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_configuration(configuration)) transformed_config = self.transform_polybox_switchdriver_config(obscured_config) with tempfile.NamedTemporaryFile(mode="w+", delete=False, encoding="utf-8") as f: @@ -71,6 +99,8 @@ async def test_connection( f.close() args = [ "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/components/renku_data_services/storage/rclone_patches.py b/components/renku_data_services/storage/rclone_patches.py index 9634ed8b1..4584a6b27 100644 --- a/components/renku_data_services/storage/rclone_patches.py +++ b/components/renku_data_services/storage/rclone_patches.py @@ -257,6 +257,66 @@ def __patch_schema_remove_banned_sftp_options(spec: list[dict[str, Any]]) -> Non sftp["Options"] = options +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, + "Examples": [ + { + "Value": "openbis-eln-lims.ethz.ch", + "Help": "Public openBIS demo instance", + "Provider": "", + }, + ], + "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": True, + "IsPassword": True, + "NoPrefix": False, + "Advanced": False, + "Exclusive": False, + "Sensitive": True, + "DefaultStr": "", + "ValueStr": "", + "Type": "string", + }, + ], + "CommandHelp": None, + "Aliases": None, + "Hide": False, + "MetadataInfo": None, + } + ) + + def apply_patches(spec: list[dict[str, Any]]) -> None: """Apply patches to RClone schema.""" patches = [ @@ -268,6 +328,7 @@ def apply_patches(spec: list[dict[str, Any]]) -> None: __patch_polybox_storage, __patch_switchdrive_storage, __patch_schema_remove_banned_sftp_options, + __patch_schema_add_openbis_type, ] for patch in patches: diff --git a/components/renku_data_services/users/api.spec.yaml b/components/renku_data_services/users/api.spec.yaml index b7fc0e794..4bb36fd2d 100644 --- a/components/renku_data_services/users/api.spec.yaml +++ b/components/renku_data_services/users/api.spec.yaml @@ -445,12 +445,14 @@ components: $ref: "#/components/schemas/Ulid" name: $ref: "#/components/schemas/SecretName" - default_filename: - $ref: "#/components/schemas/SecretDefaultFilename" - modification_date: - $ref: "#/components/schemas/ModificationDate" kind: $ref: "#/components/schemas/SecretKind" + expiration_timestamp: + $ref: "#/components/schemas/ExpirationTimestamp" + modification_date: + $ref: "#/components/schemas/ModificationDate" + default_filename: + $ref: "#/components/schemas/SecretDefaultFilename" session_secret_slot_ids: $ref: "#/components/schemas/UlidList" data_connector_ids: @@ -470,8 +472,6 @@ components: properties: name: $ref: "#/components/schemas/SecretName" - default_filename: - $ref: "#/components/schemas/SecretDefaultFilename" value: $ref: "#/components/schemas/SecretValue" kind: @@ -479,6 +479,10 @@ components: - $ref: "#/components/schemas/SecretKind" - default: "general" default: general + expiration_timestamp: + $ref: "#/components/schemas/ExpirationTimestamp" + default_filename: + $ref: "#/components/schemas/SecretDefaultFilename" required: - "name" - "value" @@ -489,10 +493,12 @@ components: properties: name: $ref: "#/components/schemas/SecretName" - default_filename: - $ref: "#/components/schemas/SecretDefaultFilename" value: $ref: "#/components/schemas/SecretValue" + expiration_timestamp: + $ref: "#/components/schemas/ExpirationTimestamp" + default_filename: + $ref: "#/components/schemas/SecretDefaultFilename" SecretName: description: The name of a user secret type: string @@ -536,6 +542,13 @@ components: ShowProjectMigrationBanner: description: Should display project migration banner type: boolean + 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 f35199909..9bb0aa1f6 100644 --- a/components/renku_data_services/users/apispec.py +++ b/components/renku_data_services/users/apispec.py @@ -208,6 +208,12 @@ class SecretWithId(BaseAPISpec): max_length=99, min_length=1, ) + 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", + ) default_filename: str = Field( ..., description="Filename to give to this secret when mounted in Renku 1.0 sessions\n", @@ -237,6 +243,18 @@ class SecretPost(BaseAPISpec): max_length=99, min_length=1, ) + value: str = Field( + ..., + description="Secret value that can be any text", + max_length=5000, + 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", + ) default_filename: Optional[str] = Field( None, description="Filename to give to this secret when mounted in Renku 1.0 sessions\n", @@ -245,13 +263,6 @@ class SecretPost(BaseAPISpec): min_length=1, pattern="^[a-zA-Z0-9_\\-.]*$", ) - value: str = Field( - ..., - description="Secret value that can be any text", - max_length=5000, - min_length=1, - ) - kind: SecretKind = SecretKind.general class SecretPatch(BaseAPISpec): @@ -265,6 +276,17 @@ class SecretPatch(BaseAPISpec): max_length=99, min_length=1, ) + value: Optional[str] = Field( + None, + description="Secret value that can be any text", + 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", + ) default_filename: Optional[str] = Field( None, description="Filename to give to this secret when mounted in Renku 1.0 sessions\n", @@ -273,12 +295,6 @@ class SecretPatch(BaseAPISpec): min_length=1, pattern="^[a-zA-Z0-9_\\-.]*$", ) - value: Optional[str] = Field( - None, - description="Secret value that can be any text", - max_length=5000, - min_length=1, - ) class PinnedProjects(BaseAPISpec): diff --git a/components/renku_data_services/users/blueprints.py b/components/renku_data_services/users/blueprints.py index 73eadc116..878478bb4 100644 --- a/components/renku_data_services/users/blueprints.py +++ b/components/renku_data_services/users/blueprints.py @@ -152,6 +152,7 @@ async def _get_all( return validated_json( apispec.SecretsList, [self._dump_secret(s) for s in secrets], + exclude_none=False, ) return "/user/secrets", ["GET"], _get_all @@ -166,6 +167,7 @@ async def _get_one(_: Request, user: base_models.APIUser, secret_id: ULID) -> JS return validated_json( apispec.SecretWithId, self._dump_secret(secret), + exclude_none=False, ) return "/user/secrets/", ["GET"], _get_one @@ -179,7 +181,9 @@ def post(self) -> BlueprintFactoryResponse: async def _post(_: Request, user: base_models.APIUser, body: apispec.SecretPost) -> JSONResponse: new_secret = validate_unsaved_secret(body) inserted_secret = await self.secret_repo.insert_secret(requested_by=user, secret=new_secret) - return validated_json(apispec.SecretWithId, self._dump_secret(inserted_secret), status=201) + return validated_json( + apispec.SecretWithId, self._dump_secret(inserted_secret), exclude_none=False, status=201 + ) return "/user/secrets", ["POST"], _post @@ -199,6 +203,7 @@ async def _patch( return validated_json( apispec.SecretWithId, self._dump_secret(updated_secret), + exclude_none=False, ) return "/user/secrets/", ["PATCH"], _patch @@ -223,6 +228,7 @@ def _dump_secret(secret: Secret) -> dict[str, Any]: default_filename=secret.default_filename, kind=secret.kind.value, modification_date=secret.modification_date, + expiration_timestamp=secret.expiration_timestamp, session_secret_slot_ids=[str(item) for item in secret.session_secret_slot_ids], data_connector_ids=[str(item) for item in secret.data_connector_ids], ) diff --git a/components/renku_data_services/users/core.py b/components/renku_data_services/users/core.py index 3f2c790b9..b85318f02 100644 --- a/components/renku_data_services/users/core.py +++ b/components/renku_data_services/users/core.py @@ -9,9 +9,10 @@ def validate_unsaved_secret(body: apispec.SecretPost) -> UnsavedSecret: secret_kind = SecretKind(body.kind.value) return UnsavedSecret( name=body.name, - default_filename=body.default_filename, secret_value=body.value, kind=secret_kind, + expiration_timestamp=body.expiration_timestamp, + default_filename=body.default_filename, ) @@ -19,6 +20,7 @@ def validate_secret_patch(patch: apispec.SecretPatch) -> SecretPatch: """Validate the update to a secret.""" return SecretPatch( name=patch.name, - default_filename=patch.default_filename, secret_value=patch.value, + expiration_timestamp=patch.expiration_timestamp, + default_filename=patch.default_filename, ) diff --git a/components/renku_data_services/utils/core.py b/components/renku_data_services/utils/core.py index 0b0fd8d75..bdf4423bc 100644 --- a/components/renku_data_services/utils/core.py +++ b/components/renku_data_services/utils/core.py @@ -4,8 +4,10 @@ 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 from deepmerge import Merger from sqlalchemy.ext.asyncio import AsyncSession @@ -78,3 +80,81 @@ async def transaction_wrapper(self: _WithSessionMaker, *args: _P.args, **kwargs: return await f(self, *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 = "AnonymousUser", + password: str | None = None, + timeout: int = 12, +) -> str: + """Requests an openBIS session token with the user's login credentials.""" + if username == "AnonymousUser" and password is None: + login = {"method": "loginAsAnonymousUser", "params": [], "id": "1", "jsonrpc": "2.0"} + else: + assert password + login = {"method": "login", "params": [username, password], "id": "2", "jsonrpc": "2.0"} + + 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() + 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( + host: str, + session_id: str, + personal_access_token_session_name: str = "renku", + minimum_validity_in_days: int = 2, + timeout: int = 12, +) -> tuple[str, datetime]: + """Requests an openBIS PAT with an openBIS session ID.""" + url = _get_url(host) + + 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: + json1: dict[str, dict[str, str]] = response.json() + 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/__snapshots__/test_storage.ambr b/test/bases/renku_data_services/data_api/__snapshots__/test_storage.ambr index 1d89d4042..5399dc6a8 100644 --- a/test/bases/renku_data_services/data_api/__snapshots__/test_storage.ambr +++ b/test/bases/renku_data_services/data_api/__snapshots__/test_storage.ambr @@ -18347,5 +18347,49 @@ ]), 'prefix': 'switchDrive', }), + dict({ + 'description': 'openBIS', + 'name': 'openbis', + 'options': list([ + dict({ + 'advanced': False, + 'default': '', + 'default_str': '', + 'examples': list([ + dict({ + 'help': 'Public openBIS demo instance', + 'provider': '', + 'value': 'openbis-eln-lims.ethz.ch', + }), + ]), + 'exclusive': False, + 'help': ''' + openBIS host to connect to. + + E.g. "openbis-eln-lims.ethz.ch". + ''', + 'ispassword': False, + 'name': 'host', + 'provider': '', + 'required': True, + 'sensitive': False, + 'type': 'string', + }), + dict({ + 'advanced': False, + 'default': '', + 'default_str': '', + 'exclusive': False, + 'help': 'openBIS session token', + 'ispassword': True, + 'name': 'session_token', + 'provider': '', + 'required': True, + 'sensitive': True, + 'type': 'string', + }), + ]), + 'prefix': 'openbis', + }), ]) # --- diff --git a/test/bases/renku_data_services/data_api/conftest.py b/test/bases/renku_data_services/data_api/conftest.py index 0ea53fd8a..a74480fa2 100644 --- a/test/bases/renku_data_services/data_api/conftest.py +++ b/test/bases/renku_data_services/data_api/conftest.py @@ -615,6 +615,39 @@ async def create_dc_helper(name: str, user: UserInfo | None = None, **payload) - return create_dc_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.path.serialize(), + "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( regular_user, user_headers, admin_user, admin_headers, create_data_connector, link_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 3674a332b..d3728a23e 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 @@ -13,6 +13,7 @@ from renku_data_services.namespace.models import NamespaceKind from renku_data_services.storage.rclone import RCloneDOIMetadata 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 if TYPE_CHECKING: @@ -1486,6 +1487,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 + # Check that the data connector is referenced from the first user secret user_secret_id = secrets[0]["secret_id"] _, response = await sanic_client.get(f"/api/data/user/secrets/{user_secret_id}", headers=user_headers) @@ -1563,7 +1572,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 @@ -1573,7 +1582,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 @@ -1583,15 +1592,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 @@ -1631,6 +1639,57 @@ 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="AnonymousUser", + ) + 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}, + ] + + def check_response(r): + assert r.status_code == 200, r.json + assert {s["name"] for s in r.json} == {"session_token"} + created_secret_ids = {s["secret_id"] for s in r.json} + assert len(created_secret_ids) == 1 + assert r.json[0].keys() == {"secret_id", "name"} + + _, response = await sanic_client.patch( + f"/api/data/data_connectors/{data_connector_id}/secrets", headers=user_headers, json=payload + ) + check_response(response) + + _, response = await sanic_client.get(f"/api/data/data_connectors/{data_connector_id}/secrets", headers=user_headers) + check_response(response) + + +@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_secret.py b/test/bases/renku_data_services/data_api/test_secret.py index 02e09ed68..168a9c724 100644 --- a/test/bases/renku_data_services/data_api/test_secret.py +++ b/test/bases/renku_data_services/data_api/test_secret.py @@ -1,7 +1,8 @@ """Tests for secrets blueprints.""" +import time from base64 import b64decode -from datetime import UTC, datetime +from datetime import UTC, datetime, timedelta from typing import Any import pytest @@ -26,9 +27,13 @@ @pytest.fixture def create_secret(sanic_client: SanicASGITestClient, user_headers): async def create_secret_helper( - name: str, value: str, kind: str = "general", default_filename: str | None = None + name: str, + value: str, + kind: str = "general", + expiration_timestamp: str = None, + default_filename: str | None = None, ) -> dict[str, Any]: - payload = {"name": name, "value": value, "kind": kind} + payload = {"name": name, "value": value, "kind": kind, "expiration_timestamp": expiration_timestamp} if default_filename: payload["default_filename"] = default_filename @@ -55,17 +60,48 @@ async def test_create_secrets(sanic_client: SanicASGITestClient, user_headers, k assert response.json.keys() == { "id", "name", - "default_filename", - "modification_date", "kind", + "expiration_timestamp", + "modification_date", + "default_filename", "session_secret_slot_ids", "data_connector_ids", } assert response.json["id"] is not None assert response.json["name"] == "my-secret" - assert response.json["default_filename"] is not None + assert response.json["kind"] == kind + assert response.json["expiration_timestamp"] is None assert response.json["modification_date"] is not None + assert response.json["default_filename"] 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", + "default_filename", + "session_secret_slot_ids", + "data_connector_ids", + } + 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 @@ -74,15 +110,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 - _, response = await sanic_client.get(f"/api/data/user/secrets/{secret_id}", headers=user_headers) +@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_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 @@ -99,6 +156,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") @@ -129,14 +202,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"} @@ -148,18 +217,37 @@ 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=payload) + _, response = await sanic_client.patch( + f"/api/data/user/secrets/{secret["id"]}", headers=user_headers, json={"name": "secret-2", "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 @@ -171,15 +259,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.py b/test/bases/renku_data_services/data_api/test_storage.py index 82dabe66b..5277bd2da 100644 --- a/test/bases/renku_data_services/data_api/test_storage.py +++ b/test/bases/renku_data_services/data_api/test_storage.py @@ -13,6 +13,7 @@ from renku_data_services.migrations.core import run_migrations_for_app from renku_data_services.storage.rclone import RCloneValidator from renku_data_services.storage.rclone_patches import BANNED_SFTP_OPTIONS, BANNED_STORAGE, OAUTH_PROVIDERS +from renku_data_services.utils.core import get_openbis_session_token from test.utils import SanicReusableASGITestClient _valid_storage: dict[str, Any] = { @@ -590,7 +591,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 @@ -599,6 +600,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 diff --git a/test/conftest.py b/test/conftest.py index 41d84c282..5f2e38f5f 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -469,3 +469,15 @@ async def solr_search(solr_config, app_manager): assert result.migrations_run == len(entity_schema.all_migrations) return solr_config + + +@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, + )