diff --git a/api/specs/web-server/_auth_api_keys.py b/api/specs/web-server/_auth_api_keys.py index 664a5165977a..bcebb0423767 100644 --- a/api/specs/web-server/_auth_api_keys.py +++ b/api/specs/web-server/_auth_api_keys.py @@ -9,8 +9,8 @@ from models_library.generics import Envelope from models_library.rest_error import EnvelopedError from simcore_service_webserver._meta import API_VTAG -from simcore_service_webserver.api_keys._controller_rest import ApiKeysPathParams -from simcore_service_webserver.api_keys._controller_rest_exceptions import ( +from simcore_service_webserver.api_keys._controller.rest import ApiKeysPathParams +from simcore_service_webserver.api_keys._controller.rest_exceptions import ( _TO_HTTP_ERROR_MAP, ) diff --git a/packages/models-library/src/models_library/rpc/webserver/auth/api_keys.py b/packages/models-library/src/models_library/rpc/webserver/auth/api_keys.py index be36327abe74..80d248d90456 100644 --- a/packages/models-library/src/models_library/rpc/webserver/auth/api_keys.py +++ b/packages/models-library/src/models_library/rpc/webserver/auth/api_keys.py @@ -1,9 +1,32 @@ import datetime as dt -from typing import Annotated +import hashlib +import re +import secrets +import string +from typing import Annotated, Final from models_library.basic_types import IDStr from pydantic import BaseModel, ConfigDict, Field +_PUNCTUATION_REGEX = re.compile( + pattern="[" + re.escape(string.punctuation.replace("_", "")) + "]" +) + +_KEY_LEN: Final = 10 +_SECRET_LEN: Final = 20 + + +def generate_unique_api_key(name: str, length: int = _KEY_LEN) -> str: + prefix = _PUNCTUATION_REGEX.sub("_", name[:5]) + hashed = hashlib.sha256(name.encode()).hexdigest() + return f"{prefix}_{hashed[:length]}" + + +def generate_api_key_and_secret(name: str): + api_key = generate_unique_api_key(name) + api_secret = secrets.token_hex(_SECRET_LEN) + return api_key, api_secret + class ApiKeyCreate(BaseModel): display_name: Annotated[str, Field(..., min_length=3)] diff --git a/packages/postgres-database/src/simcore_postgres_database/migration/versions/742123f0933a_hash_exising_api_secret_data.py b/packages/postgres-database/src/simcore_postgres_database/migration/versions/742123f0933a_hash_exising_api_secret_data.py new file mode 100644 index 000000000000..e223c24a6157 --- /dev/null +++ b/packages/postgres-database/src/simcore_postgres_database/migration/versions/742123f0933a_hash_exising_api_secret_data.py @@ -0,0 +1,28 @@ +"""hash existing api_secret data + +Revision ID: 742123f0933a +Revises: b0c988e3f348 +Create Date: 2025-03-13 09:39:43.895529+00:00 + +""" + +from alembic import op + +# revision identifiers, used by Alembic. +revision = "742123f0933a" +down_revision = "b0c988e3f348" +branch_labels = None +depends_on = None + + +def upgrade(): + op.execute( + """ + UPDATE api_keys + SET api_secret = crypt(api_secret, gen_salt('bf', 10)) + """ + ) + + +def downgrade(): + pass diff --git a/packages/postgres-database/src/simcore_postgres_database/migration/versions/b0c988e3f348_add_index_to_api_key_column.py b/packages/postgres-database/src/simcore_postgres_database/migration/versions/b0c988e3f348_add_index_to_api_key_column.py new file mode 100644 index 000000000000..378d595c0715 --- /dev/null +++ b/packages/postgres-database/src/simcore_postgres_database/migration/versions/b0c988e3f348_add_index_to_api_key_column.py @@ -0,0 +1,23 @@ +"""add index to api_key column + +Revision ID: b0c988e3f348 +Revises: f65f7786cd4b +Create Date: 2025-03-13 08:53:05.722855+00:00 + +""" + +from alembic import op + +# revision identifiers, used by Alembic. +revision = "b0c988e3f348" +down_revision = "f65f7786cd4b" +branch_labels = None +depends_on = None + + +def upgrade(): + op.create_index(op.f("ix_api_keys_api_key"), "api_keys", ["api_key"], unique=False) + + +def downgrade(): + op.drop_index(op.f("ix_api_keys_api_key"), table_name="api_keys") diff --git a/packages/postgres-database/src/simcore_postgres_database/models/api_keys.py b/packages/postgres-database/src/simcore_postgres_database/models/api_keys.py index 416e29d4e2df..2c3f12eca3ab 100644 --- a/packages/postgres-database/src/simcore_postgres_database/models/api_keys.py +++ b/packages/postgres-database/src/simcore_postgres_database/models/api_keys.py @@ -1,4 +1,4 @@ -""" API keys to access public API +"""API keys to access public API These keys grant the client authorization to the API resources @@ -10,6 +10,7 @@ +--------+ +---------------+ """ + import sqlalchemy as sa from sqlalchemy.sql import func @@ -52,8 +53,13 @@ nullable=False, doc="Identified product", ), - sa.Column("api_key", sa.String(), nullable=False), - sa.Column("api_secret", sa.String(), nullable=False), + sa.Column("api_key", sa.String(), nullable=False, index=True), + sa.Column( + "api_secret", + sa.String(), + nullable=False, + doc="API key secret, hashed using blowfish algorithm", + ), sa.Column( "created", sa.DateTime(), diff --git a/packages/postgres-database/tests/test_models_api_keys.py b/packages/postgres-database/tests/test_models_api_keys.py index 2c5de1a56a8d..d8863f9ac748 100644 --- a/packages/postgres-database/tests/test_models_api_keys.py +++ b/packages/postgres-database/tests/test_models_api_keys.py @@ -10,7 +10,7 @@ from aiopg.sa.connection import SAConnection from aiopg.sa.result import RowProxy from pytest_simcore.helpers.faker_factories import ( - random_api_key, + random_api_auth, random_product, random_user, ) @@ -48,7 +48,7 @@ async def test_create_and_delete_api_key( ): apikey_id = await connection.scalar( api_keys.insert() - .values(**random_api_key(product_name, user_id)) + .values(**random_api_auth(product_name, user_id)) .returning(api_keys.c.id) ) @@ -66,7 +66,7 @@ async def session_auth( # uses to authenticate a session. result = await connection.execute( api_keys.insert() - .values(**random_api_key(product_name, user_id)) + .values(**random_api_auth(product_name, user_id)) .returning(sa.literal_column("*")) ) row = await result.fetchone() diff --git a/packages/pytest-simcore/src/pytest_simcore/helpers/faker_factories.py b/packages/pytest-simcore/src/pytest_simcore/helpers/faker_factories.py index 04cde49a9fff..6ff20f1b072d 100644 --- a/packages/pytest-simcore/src/pytest_simcore/helpers/faker_factories.py +++ b/packages/pytest-simcore/src/pytest_simcore/helpers/faker_factories.py @@ -405,7 +405,7 @@ def random_payment_autorecharge( return data -def random_api_key( +def random_api_auth( product_name: str, user_id: int, fake: Faker = DEFAULT_FAKER, **overrides ) -> dict[str, Any]: from simcore_postgres_database.models.api_keys import api_keys diff --git a/packages/service-library/src/servicelib/rabbitmq/rpc_interfaces/webserver/auth/api_keys.py b/packages/service-library/src/servicelib/rabbitmq/rpc_interfaces/webserver/auth/api_keys.py index 2609de81c5e3..0358a0e3b6ad 100644 --- a/packages/service-library/src/servicelib/rabbitmq/rpc_interfaces/webserver/auth/api_keys.py +++ b/packages/service-library/src/servicelib/rabbitmq/rpc_interfaces/webserver/auth/api_keys.py @@ -4,6 +4,7 @@ from models_library.basic_types import IDStr from models_library.rabbitmq_basic_types import RPCMethodName from models_library.rpc.webserver.auth.api_keys import ApiKeyCreate, ApiKeyGet +from models_library.users import UserID from pydantic import TypeAdapter from servicelib.logging_utils import log_decorator from servicelib.rabbitmq import RabbitMQRPCClient @@ -15,7 +16,7 @@ async def create_api_key( rabbitmq_rpc_client: RabbitMQRPCClient, *, - user_id: str, + user_id: UserID, product_name: str, api_key: ApiKeyCreate, ) -> ApiKeyGet: @@ -24,7 +25,8 @@ async def create_api_key( TypeAdapter(RPCMethodName).validate_python("create_api_key"), user_id=user_id, product_name=product_name, - api_key=api_key, + display_name=api_key.display_name, + expiration=api_key.expiration, ) assert isinstance(result, ApiKeyGet) # nosec return result @@ -34,7 +36,7 @@ async def create_api_key( async def get_api_key( rabbitmq_rpc_client: RabbitMQRPCClient, *, - user_id: str, + user_id: UserID, product_name: str, api_key_id: IDStr, ) -> ApiKeyGet: @@ -49,18 +51,19 @@ async def get_api_key( return result -async def delete_api_key( +@log_decorator(_logger, level=logging.DEBUG) +async def delete_api_key_by_key( rabbitmq_rpc_client: RabbitMQRPCClient, *, - user_id: str, + user_id: UserID, product_name: str, - api_key_id: IDStr, + api_key: str, ) -> None: result = await rabbitmq_rpc_client.request( WEBSERVER_RPC_NAMESPACE, - TypeAdapter(RPCMethodName).validate_python("delete_api_key"), + TypeAdapter(RPCMethodName).validate_python("delete_api_key_by_key"), user_id=user_id, product_name=product_name, - api_key_id=api_key_id, + api_key=api_key, ) assert result is None # nosec diff --git a/services/api-server/src/simcore_service_api_server/db/repositories/api_keys.py b/services/api-server/src/simcore_service_api_server/db/repositories/api_keys.py index 400b4e8b1c14..5d70470f0251 100644 --- a/services/api-server/src/simcore_service_api_server/db/repositories/api_keys.py +++ b/services/api-server/src/simcore_service_api_server/db/repositories/api_keys.py @@ -22,8 +22,11 @@ async def get_user( self, api_key: str, api_secret: str ) -> UserAndProductTuple | None: stmt = sa.select(tbl.api_keys.c.user_id, tbl.api_keys.c.product_name).where( - (tbl.api_keys.c.api_key == api_key) - & (tbl.api_keys.c.api_secret == api_secret), + (tbl.api_keys.c.api_key == api_key) # NOTE: keep order, api_key is indexed + & ( + tbl.api_keys.c.api_secret + == sa.func.crypt(api_secret, tbl.api_keys.c.api_secret) + ) ) result: UserAndProductTuple | None = None try: diff --git a/services/api-server/tests/unit/_with_db/conftest.py b/services/api-server/tests/unit/_with_db/conftest.py index dc160808f310..3d1bd4a016f2 100644 --- a/services/api-server/tests/unit/_with_db/conftest.py +++ b/services/api-server/tests/unit/_with_db/conftest.py @@ -25,7 +25,7 @@ from pytest_mock import MockerFixture from pytest_simcore.helpers import postgres_tools from pytest_simcore.helpers.faker_factories import ( - random_api_key, + random_api_auth, random_product, random_user, ) @@ -255,31 +255,39 @@ async def create_fake_api_keys( create_product_names: Callable[[PositiveInt], AsyncGenerator[str, None]], ) -> AsyncGenerator[Callable[[PositiveInt], AsyncGenerator[ApiKeyInDB, None]], None]: async def _generate_fake_api_key(n: PositiveInt): - users = create_user_ids(n) - products = create_product_names(n) + users, products = create_user_ids(n), create_product_names(n) + excluded_column = "api_secret" + returning_cols = [col for col in api_keys.c if col.name != excluded_column] + for _ in range(n): product = await anext(products) user = await anext(users) + api_auth = random_api_auth(product, user) + plain_api_secret = api_auth.pop("api_secret") result = await connection.execute( api_keys.insert() - .values(**random_api_key(product, user)) - .returning(sa.literal_column("*")) + .values( + api_secret=sa.func.crypt(plain_api_secret, sa.func.gen_salt("bf", 10)), + **api_auth, + ) + .returning(*returning_cols) ) row = await result.fetchone() assert row _generate_fake_api_key.row_ids.append(row.id) - yield ApiKeyInDB.model_validate(row) + yield ApiKeyInDB.model_validate({"api_secret": plain_api_secret, **row}) _generate_fake_api_key.row_ids = [] yield _generate_fake_api_key - for row_id in _generate_fake_api_key.row_ids: - await connection.execute(api_keys.delete().where(api_keys.c.id == row_id)) + await connection.execute( + api_keys.delete().where(api_keys.c.id.in_(_generate_fake_api_key.row_ids)) + ) @pytest.fixture async def auth( - create_fake_api_keys: Callable[[PositiveInt], AsyncGenerator[ApiKeyInDB, None]] + create_fake_api_keys: Callable[[PositiveInt], AsyncGenerator[ApiKeyInDB, None]], ) -> httpx.BasicAuth: """overrides auth and uses access to real repositories instead of mocks""" async for key in create_fake_api_keys(1): diff --git a/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_core/_events_utils.py b/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_core/_events_utils.py index df051c921ba0..5703459bf9c4 100644 --- a/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_core/_events_utils.py +++ b/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/scheduler/_core/_events_utils.py @@ -11,6 +11,7 @@ from models_library.projects_networks import ProjectsNetworks from models_library.projects_nodes_io import NodeID, NodeIDStr from models_library.rabbitmq_messages import InstrumentationRabbitMessage +from models_library.rpc.webserver.auth.api_keys import generate_unique_api_key from models_library.service_settings_labels import SimcoreServiceLabels from models_library.shared_user_preferences import ( AllowMetricsCollectionFrontendUserPreference, @@ -59,6 +60,7 @@ get_rate, track_duration, ) +from .....modules.osparc_variables._api_auth import create_unique_api_name_for from .....utils.db import get_repository from ....db.repositories.projects import ProjectsRepository from ....db.repositories.projects_networks import ProjectsNetworksRepository @@ -66,6 +68,7 @@ UserPreferencesFrontendRepository, ) from ....director_v0 import DirectorV0Client +from ....osparc_variables._api_auth_rpc import delete_api_key_by_key from ...api_client import ( SidecarsClient, get_dynamic_sidecar_service_health, @@ -93,7 +96,7 @@ def get_director_v0_client(app: FastAPI) -> DirectorV0Client: def parse_containers_inspect( - containers_inspect: dict[str, Any] | None + containers_inspect: dict[str, Any] | None, ) -> list[DockerContainerInspect]: if containers_inspect is None: return [] @@ -306,6 +309,24 @@ async def attempt_pod_removal_and_data_saving( await service_remove_containers(app, scheduler_data.node_uuid, sidecars_client) + if scheduler_data.product_name: + try: + display_name = create_unique_api_name_for( + scheduler_data.product_name, + scheduler_data.user_id, + scheduler_data.project_id, + scheduler_data.node_uuid, + ) + api_key = generate_unique_api_key(display_name) + await delete_api_key_by_key( + app, + product_name=scheduler_data.product_name, + user_id=scheduler_data.user_id, + api_key=api_key, + ) + except Exception: # pylint: disable=broad-except + _logger.warning("failed to delete api key %s", display_name) + # used for debuug, normally sleeps 0 await asyncio.sleep( settings.DIRECTOR_V2_DYNAMIC_SIDECAR_SLEEP_AFTER_CONTAINER_REMOVAL.total_seconds() diff --git a/services/director-v2/src/simcore_service_director_v2/modules/osparc_variables/_api_auth.py b/services/director-v2/src/simcore_service_director_v2/modules/osparc_variables/_api_auth.py index a207df3aec38..d7f95f0c6dfb 100644 --- a/services/director-v2/src/simcore_service_director_v2/modules/osparc_variables/_api_auth.py +++ b/services/director-v2/src/simcore_service_director_v2/modules/osparc_variables/_api_auth.py @@ -1,73 +1,115 @@ import uuid -from typing import cast +from datetime import timedelta from uuid import uuid5 from aiocache import cached # type: ignore[import-untyped] from fastapi import FastAPI from models_library.products import ProductName +from models_library.projects import ProjectID +from models_library.projects_nodes_io import NodeID from models_library.rpc.webserver.auth.api_keys import ApiKeyGet from models_library.users import UserID -from ._api_auth_rpc import get_or_create_api_key_and_secret +from ._api_auth_rpc import create_api_key as rpc_create_api_key +from ._api_auth_rpc import delete_api_key_by_key as rpc_delete_api_key +_EXPIRATION_AUTO_KEYS = timedelta(weeks=4) -def create_unique_api_name_for(product_name: ProductName, user_id: UserID) -> str: + +def create_unique_api_name_for( + product_name: ProductName, + user_id: UserID, + project_id: ProjectID, + node_id: NodeID, +) -> str: # NOTE: The namespace chosen doesn't significantly impact the resulting UUID # as long as it's consistently used across the same context - return f"__auto_{uuid5(uuid.NAMESPACE_DNS, f'{product_name}/{user_id}')}" + return f"__auto_{uuid5(uuid.NAMESPACE_DNS, f'{product_name}/{user_id}/{project_id}/{node_id}')}" # NOTE: Uses caching to prevent multiple calls to the external service -# when 'get_or_create_user_api_key' or 'get_or_create_user_api_secret' are invoked. +# when 'create_user_api_key' or 'create_user_api_secret' are invoked. def _cache_key(fct, *_, **kwargs): - return f"{fct.__name__}_{kwargs['product_name']}_{kwargs['user_id']}" + return f"{fct.__name__}_{kwargs['product_name']}_{kwargs['user_id']}_{kwargs['project_id']}_{kwargs['node_id']}" @cached(ttl=3, key_builder=_cache_key) -async def _get_or_create_for( +async def _create_for( app: FastAPI, *, product_name: ProductName, user_id: UserID, + project_id: ProjectID, + node_id: NodeID, ) -> ApiKeyGet: - display_name = create_unique_api_name_for(product_name, user_id) - return await get_or_create_api_key_and_secret( + display_name = create_unique_api_name_for( + product_name, user_id, project_id, node_id + ) + return await rpc_create_api_key( app, user_id=user_id, product_name=product_name, display_name=display_name, - expiration=None, + expiration=_EXPIRATION_AUTO_KEYS, ) -async def get_or_create_user_api_key( +async def create_user_api_key( app: FastAPI, product_name: ProductName, user_id: UserID, + project_id: ProjectID, + node_id: NodeID, ) -> str: - data = await _get_or_create_for( + data = await _create_for( app, product_name=product_name, user_id=user_id, + project_id=project_id, + node_id=node_id, ) - return cast(str, data.api_key) + assert data.api_key # nosec + assert isinstance(data.api_key, str) # nosec + return data.api_key -async def get_or_create_user_api_secret( +async def create_user_api_secret( app: FastAPI, product_name: ProductName, user_id: UserID, + project_id: ProjectID, + node_id: NodeID, ) -> str: - data = await _get_or_create_for( + data = await _create_for( app, product_name=product_name, user_id=user_id, + project_id=project_id, + node_id=node_id, + ) + assert data.api_secret # nosec + assert isinstance(data.api_secret, str) # nosec + return data.api_secret + + +async def delete_api_key_by_key( + app: FastAPI, + product_name: ProductName, + user_id: UserID, + project_id: ProjectID, + node_id: NodeID, +) -> None: + api_key = create_unique_api_name_for(product_name, user_id, project_id, node_id) + await rpc_delete_api_key( + app=app, + product_name=product_name, + user_id=user_id, + api_key=api_key, ) - return cast(str, data.api_secret) __all__: tuple[str, ...] = ( - "get_or_create_user_api_key", - "get_or_create_user_api_secret", - "create_unique_api_name_for", + "create_user_api_key", + "create_user_api_secret", + "delete_api_key_by_key", ) diff --git a/services/director-v2/src/simcore_service_director_v2/modules/osparc_variables/_api_auth_rpc.py b/services/director-v2/src/simcore_service_director_v2/modules/osparc_variables/_api_auth_rpc.py index 1a6da1f73822..d623305229d6 100644 --- a/services/director-v2/src/simcore_service_director_v2/modules/osparc_variables/_api_auth_rpc.py +++ b/services/director-v2/src/simcore_service_director_v2/modules/osparc_variables/_api_auth_rpc.py @@ -15,7 +15,7 @@ # -async def get_or_create_api_key_and_secret( +async def create_api_key( app: FastAPI, *, product_name: ProductName, @@ -26,10 +26,27 @@ async def get_or_create_api_key_and_secret( rpc_client = get_rabbitmq_rpc_client(app) result = await rpc_client.request( WEBSERVER_RPC_NAMESPACE, - TypeAdapter(RPCMethodName).validate_python("get_or_create_api_key"), + TypeAdapter(RPCMethodName).validate_python("create_api_key"), + product_name=product_name, user_id=user_id, display_name=display_name, expiration=expiration, - product_name=product_name, ) return ApiKeyGet.model_validate(result) + + +async def delete_api_key_by_key( + app: FastAPI, + *, + product_name: ProductName, + user_id: UserID, + api_key: str, +) -> None: + rpc_client = get_rabbitmq_rpc_client(app) + await rpc_client.request( + WEBSERVER_RPC_NAMESPACE, + TypeAdapter(RPCMethodName).validate_python("delete_api_key_by_key"), + product_name=product_name, + user_id=user_id, + api_key=api_key, + ) diff --git a/services/director-v2/src/simcore_service_director_v2/modules/osparc_variables/substitutions.py b/services/director-v2/src/simcore_service_director_v2/modules/osparc_variables/substitutions.py index d15e110689a9..00fc4c5ba0ee 100644 --- a/services/director-v2/src/simcore_service_director_v2/modules/osparc_variables/substitutions.py +++ b/services/director-v2/src/simcore_service_director_v2/modules/osparc_variables/substitutions.py @@ -32,7 +32,7 @@ resolve_variables_from_context, ) from ..db.repositories.services_environments import ServicesEnvironmentsRepository -from ._api_auth import get_or_create_user_api_key, get_or_create_user_api_secret +from ._api_auth import create_user_api_key, create_user_api_secret from ._user import request_user_email, request_user_role _logger = logging.getLogger(__name__) @@ -130,11 +130,9 @@ def create(cls, app: FastAPI): table.register_from_handler("OSPARC_VARIABLE_USER_EMAIL")(request_user_email) table.register_from_handler("OSPARC_VARIABLE_USER_ROLE")(request_user_role) - table.register_from_handler("OSPARC_VARIABLE_API_KEY")( - get_or_create_user_api_key - ) + table.register_from_handler("OSPARC_VARIABLE_API_KEY")(create_user_api_key) table.register_from_handler("OSPARC_VARIABLE_API_SECRET")( - get_or_create_user_api_secret + create_user_api_secret ) _logger.debug( diff --git a/services/director-v2/tests/conftest.py b/services/director-v2/tests/conftest.py index 01790674d091..22a36ee0f5ca 100644 --- a/services/director-v2/tests/conftest.py +++ b/services/director-v2/tests/conftest.py @@ -357,9 +357,7 @@ async def wrapper(*args, **kwargs): @pytest.fixture def mock_osparc_variables_api_auth_rpc(mocker: MockerFixture) -> None: - fake_data = ApiKeyGet.model_validate( - ApiKeyGet.model_config["json_schema_extra"]["examples"][0] - ) + fake_data = ApiKeyGet.model_validate(ApiKeyGet.model_json_schema()["examples"][0]) async def _create( app: FastAPI, @@ -372,14 +370,14 @@ async def _create( assert app assert product_name assert user_id - assert expiration is None + assert expiration fake_data.display_name = display_name return fake_data # mocks RPC interface mocker.patch( - "simcore_service_director_v2.modules.osparc_variables._api_auth.get_or_create_api_key_and_secret", + "simcore_service_director_v2.modules.osparc_variables._api_auth.rpc_create_api_key", side_effect=_create, autospec=True, ) diff --git a/services/web/server/src/simcore_service_webserver/api_keys/_controller/__init__.py b/services/web/server/src/simcore_service_webserver/api_keys/_controller/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/services/web/server/src/simcore_service_webserver/api_keys/_controller_rest.py b/services/web/server/src/simcore_service_webserver/api_keys/_controller/rest.py similarity index 90% rename from services/web/server/src/simcore_service_webserver/api_keys/_controller_rest.py rename to services/web/server/src/simcore_service_webserver/api_keys/_controller/rest.py index e85001b03a10..b7db0f0068d9 100644 --- a/services/web/server/src/simcore_service_webserver/api_keys/_controller_rest.py +++ b/services/web/server/src/simcore_service_webserver/api_keys/_controller/rest.py @@ -17,14 +17,14 @@ parse_request_path_parameters_as, ) -from .._meta import API_VTAG -from ..login.decorators import login_required -from ..models import RequestContext -from ..security.decorators import permission_required -from ..utils_aiohttp import envelope_json_response -from . import _service -from ._controller_rest_exceptions import handle_plugin_requests_exceptions -from .models import ApiKey +from ..._meta import API_VTAG +from ...login.decorators import login_required +from ...models import RequestContext +from ...security.decorators import permission_required +from ...utils_aiohttp import envelope_json_response +from .. import _service +from ..models import ApiKey +from .rest_exceptions import handle_plugin_requests_exceptions _logger = logging.getLogger(__name__) diff --git a/services/web/server/src/simcore_service_webserver/api_keys/_controller_rest_exceptions.py b/services/web/server/src/simcore_service_webserver/api_keys/_controller/rest_exceptions.py similarity index 85% rename from services/web/server/src/simcore_service_webserver/api_keys/_controller_rest_exceptions.py rename to services/web/server/src/simcore_service_webserver/api_keys/_controller/rest_exceptions.py index 4cb6c84f824d..f5888adc99ba 100644 --- a/services/web/server/src/simcore_service_webserver/api_keys/_controller_rest_exceptions.py +++ b/services/web/server/src/simcore_service_webserver/api_keys/_controller/rest_exceptions.py @@ -1,12 +1,12 @@ from servicelib.aiohttp import status -from ..exception_handling import ( +from ...exception_handling import ( ExceptionToHttpErrorMap, HttpErrorInfo, exception_handling_decorator, to_exceptions_handlers_map, ) -from .errors import ApiKeyDuplicatedDisplayNameError, ApiKeyNotFoundError +from ..errors import ApiKeyDuplicatedDisplayNameError, ApiKeyNotFoundError _TO_HTTP_ERROR_MAP: ExceptionToHttpErrorMap = { ApiKeyDuplicatedDisplayNameError: HttpErrorInfo( diff --git a/services/web/server/src/simcore_service_webserver/api_keys/_controller_rpc.py b/services/web/server/src/simcore_service_webserver/api_keys/_controller/rpc.py similarity index 67% rename from services/web/server/src/simcore_service_webserver/api_keys/_controller_rpc.py rename to services/web/server/src/simcore_service_webserver/api_keys/_controller/rpc.py index 59c91c3be59b..26a3e5afc71e 100644 --- a/services/web/server/src/simcore_service_webserver/api_keys/_controller_rpc.py +++ b/services/web/server/src/simcore_service_webserver/api_keys/_controller/rpc.py @@ -2,16 +2,15 @@ from aiohttp import web from models_library.api_schemas_webserver import WEBSERVER_RPC_NAMESPACE -from models_library.api_schemas_webserver.auth import ApiKeyCreateRequest from models_library.products import ProductName from models_library.rpc.webserver.auth.api_keys import ApiKeyGet from models_library.users import UserID from servicelib.rabbitmq import RPCRouter -from ..rabbitmq import get_rabbitmq_rpc_server -from . import _service -from .errors import ApiKeyNotFoundError -from .models import ApiKey +from ...rabbitmq import get_rabbitmq_rpc_server +from .. import _service +from ..errors import ApiKeyNotFoundError +from ..models import ApiKey router = RPCRouter() @@ -22,14 +21,15 @@ async def create_api_key( *, user_id: UserID, product_name: ProductName, - api_key: ApiKeyCreateRequest, + display_name: str, + expiration: timedelta | None = None, ) -> ApiKeyGet: created_api_key: ApiKey = await _service.create_api_key( app, user_id=user_id, product_name=product_name, - display_name=api_key.display_name, - expiration=api_key.expiration, + display_name=display_name, + expiration=expiration, ) return ApiKeyGet.model_validate(created_api_key) @@ -53,37 +53,18 @@ async def get_api_key( @router.expose() -async def get_or_create_api_key( +async def delete_api_key_by_key( app: web.Application, *, user_id: UserID, product_name: ProductName, - display_name: str, - expiration: timedelta | None = None, -) -> ApiKeyGet: - api_key: ApiKey = await _service.get_or_create_api_key( - app, - user_id=user_id, - product_name=product_name, - display_name=display_name, - expiration=expiration, - ) - return ApiKeyGet.model_validate(api_key) - - -@router.expose() -async def delete_api_key( - app: web.Application, - *, - user_id: UserID, - product_name: ProductName, - api_key_id: str, + api_key: str, ) -> None: - await _service.delete_api_key( + await _service.delete_api_key_by_key( app, user_id=user_id, product_name=product_name, - api_key_id=api_key_id, + api_key=api_key, ) diff --git a/services/web/server/src/simcore_service_webserver/api_keys/_repository.py b/services/web/server/src/simcore_service_webserver/api_keys/_repository.py index bba0097403d9..72e58d7bcf18 100644 --- a/services/web/server/src/simcore_service_webserver/api_keys/_repository.py +++ b/services/web/server/src/simcore_service_webserver/api_keys/_repository.py @@ -3,7 +3,6 @@ import sqlalchemy as sa from aiohttp import web -from asyncpg.exceptions import UniqueViolationError from models_library.products import ProductName from models_library.users import UserID from simcore_postgres_database.models.api_keys import api_keys @@ -15,53 +14,16 @@ from sqlalchemy.ext.asyncio import AsyncConnection from ..db.plugin import get_asyncpg_engine -from .errors import ApiKeyDuplicatedDisplayNameError from .models import ApiKey _logger = logging.getLogger(__name__) -async def create_api_key( - app: web.Application, - connection: AsyncConnection | None = None, - *, - user_id: UserID, - product_name: ProductName, - display_name: str, - expiration: timedelta | None, - api_key: str, - api_secret: str, -) -> ApiKey: - async with transaction_context(get_asyncpg_engine(app), connection) as conn: - try: - stmt = ( - api_keys.insert() - .values( - display_name=display_name, - user_id=user_id, - product_name=product_name, - api_key=api_key, - api_secret=api_secret, - expires_at=(sa.func.now() + expiration) if expiration else None, - ) - .returning(api_keys.c.id) - ) +def _hash_secret(secret: str) -> sa.sql.ClauseElement: + return sa.func.crypt(secret, sa.func.gen_salt("bf", 10)) - result = await conn.stream(stmt) - row = await result.one() - return ApiKey( - id=f"{row.id}", # NOTE See: https://github.com/ITISFoundation/osparc-simcore/issues/6919 - display_name=display_name, - expiration=expiration, - api_key=api_key, - api_secret=api_secret, - ) - except UniqueViolationError as exc: - raise ApiKeyDuplicatedDisplayNameError(display_name=display_name) from exc - - -async def get_or_create_api_key( +async def create_api_key( app: web.Application, connection: AsyncConnection | None = None, *, @@ -73,62 +35,90 @@ async def get_or_create_api_key( api_secret: str, ) -> ApiKey: async with transaction_context(get_asyncpg_engine(app), connection) as conn: - # Implemented as "create or get" - insert_stmt = ( + stmt = ( pg_insert(api_keys) .values( display_name=display_name, user_id=user_id, product_name=product_name, api_key=api_key, - api_secret=api_secret, + api_secret=_hash_secret(api_secret), expires_at=(sa.func.now() + expiration) if expiration else None, ) .on_conflict_do_update( index_elements=["user_id", "display_name"], set_={ - "product_name": product_name - }, # dummy enable returning since on_conflict_do_nothing returns None - # NOTE: use this entry for reference counting in https://github.com/ITISFoundation/osparc-simcore/issues/5875 + "api_key": api_key, + "api_secret": _hash_secret(api_secret), + "expires_at": (sa.func.now() + expiration) if expiration else None, + }, ) .returning(api_keys) ) - result = await conn.stream(insert_stmt) - row = await result.first() - assert row # nosec + result = await conn.stream(stmt) + row = await result.one() return ApiKey( id=f"{row.id}", # NOTE See: https://github.com/ITISFoundation/osparc-simcore/issues/6919 - display_name=row.display_name, - expiration=row.expires_at, - api_key=row.api_key, - api_secret=row.api_secret, + display_name=display_name, + expiration=expiration, + api_key=api_key, + api_secret=api_secret, ) -async def list_api_keys( +async def delete_api_key( app: web.Application, connection: AsyncConnection | None = None, *, user_id: UserID, product_name: ProductName, -) -> list[ApiKey]: - async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: - stmt = sa.select(api_keys.c.id, api_keys.c.display_name).where( - (api_keys.c.user_id == user_id) & (api_keys.c.product_name == product_name) + api_key_id: str, +) -> None: + async with transaction_context(get_asyncpg_engine(app), connection) as conn: + stmt = api_keys.delete().where( + ( + api_keys.c.id == int(api_key_id) + ) # NOTE See: https://github.com/ITISFoundation/osparc-simcore/issues/6919 + & (api_keys.c.user_id == user_id) + & (api_keys.c.product_name == product_name) ) + await conn.execute(stmt) - result = await conn.stream(stmt) - rows = [row async for row in result] - return [ - ApiKey( - id=f"{row.id}", # NOTE See: https://github.com/ITISFoundation/osparc-simcore/issues/6919 - display_name=row.display_name, +async def delete_api_key_by_key( + app: web.Application, + connection: AsyncConnection | None = None, + *, + product_name: ProductName, + user_id: UserID, + api_key: str, +) -> None: + async with transaction_context(get_asyncpg_engine(app), connection) as conn: + stmt = api_keys.delete().where( + (api_keys.c.api_key == api_key) + & (api_keys.c.user_id == user_id) + & (api_keys.c.product_name == product_name) + ) + await conn.execute(stmt) + + +async def delete_expired_api_keys( + app: web.Application, connection: AsyncConnection | None = None +) -> list[str]: + async with transaction_context(get_asyncpg_engine(app), connection) as conn: + stmt = ( + api_keys.delete() + .where( + (api_keys.c.expires_at.is_not(None)) + & (api_keys.c.expires_at < sa.func.now()) ) - for row in rows - ] + .returning(api_keys.c.display_name) + ) + result = await conn.execute(stmt) + rows = result.fetchall() + return [r.display_name for r in rows] async def get_api_key( @@ -164,37 +154,25 @@ async def get_api_key( ) -async def delete_api_key( +async def list_api_keys( app: web.Application, connection: AsyncConnection | None = None, *, user_id: UserID, product_name: ProductName, - api_key_id: str, -) -> None: - async with transaction_context(get_asyncpg_engine(app), connection) as conn: - stmt = api_keys.delete().where( - ( - api_keys.c.id == int(api_key_id) - ) # NOTE See: https://github.com/ITISFoundation/osparc-simcore/issues/6919 - & (api_keys.c.user_id == user_id) - & (api_keys.c.product_name == product_name) +) -> list[ApiKey]: + async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn: + stmt = sa.select(api_keys.c.id, api_keys.c.display_name).where( + (api_keys.c.user_id == user_id) & (api_keys.c.product_name == product_name) ) - await conn.execute(stmt) + result = await conn.stream(stmt) + rows = [row async for row in result] -async def delete_expired_api_keys( - app: web.Application, connection: AsyncConnection | None = None -) -> list[str]: - async with transaction_context(get_asyncpg_engine(app), connection) as conn: - stmt = ( - api_keys.delete() - .where( - (api_keys.c.expires_at.is_not(None)) - & (api_keys.c.expires_at < sa.func.now()) + return [ + ApiKey( + id=f"{row.id}", # NOTE See: https://github.com/ITISFoundation/osparc-simcore/issues/6919 + display_name=row.display_name, ) - .returning(api_keys.c.display_name) - ) - result = await conn.execute(stmt) - rows = result.fetchall() - return [r.display_name for r in rows] + for row in rows + ] diff --git a/services/web/server/src/simcore_service_webserver/api_keys/_service.py b/services/web/server/src/simcore_service_webserver/api_keys/_service.py index d5648e43060c..93e8c3610942 100644 --- a/services/web/server/src/simcore_service_webserver/api_keys/_service.py +++ b/services/web/server/src/simcore_service_webserver/api_keys/_service.py @@ -1,31 +1,16 @@ import datetime as dt -import re -import string -from typing import Final from aiohttp import web from models_library.products import ProductName +from models_library.rpc.webserver.auth.api_keys import ( + generate_api_key_and_secret, +) from models_library.users import UserID -from servicelib.utils_secrets import generate_token_secret_key from . import _repository from .errors import ApiKeyNotFoundError from .models import ApiKey -_PUNCTUATION_REGEX = re.compile( - pattern="[" + re.escape(string.punctuation.replace("_", "")) + "]" -) - -_KEY_LEN: Final = 10 -_SECRET_LEN: Final = 20 - - -def _generate_api_key_and_secret(name: str): - prefix = _PUNCTUATION_REGEX.sub("_", name[:5]) - api_key = f"{prefix}_{generate_token_secret_key(_KEY_LEN)}" - api_secret = generate_token_secret_key(_SECRET_LEN) - return api_key, api_secret - async def create_api_key( app: web.Application, @@ -35,7 +20,7 @@ async def create_api_key( display_name: str, expiration: dt.timedelta | None, ) -> ApiKey: - api_key, api_secret = _generate_api_key_and_secret(display_name) + api_key, api_secret = generate_api_key_and_secret(display_name) return await _repository.create_api_key( app, @@ -48,74 +33,65 @@ async def create_api_key( ) -async def list_api_keys( +async def delete_api_key( app: web.Application, *, - user_id: UserID, product_name: ProductName, -) -> list[ApiKey]: - api_keys: list[ApiKey] = await _repository.list_api_keys( - app, user_id=user_id, product_name=product_name + user_id: UserID, + api_key_id: str, +) -> None: + await _repository.delete_api_key( + app, + api_key_id=api_key_id, + user_id=user_id, + product_name=product_name, ) - return api_keys -async def get_api_key( +async def delete_api_key_by_key( app: web.Application, *, - api_key_id: str, - user_id: UserID, product_name: ProductName, -) -> ApiKey: - api_key: ApiKey | None = await _repository.get_api_key( + user_id: UserID, + api_key: str, +) -> None: + await _repository.delete_api_key_by_key( app, - api_key_id=api_key_id, - user_id=user_id, product_name=product_name, + user_id=user_id, + api_key=api_key, ) - if api_key is not None: - return api_key - - raise ApiKeyNotFoundError(api_key_id=api_key_id) -async def get_or_create_api_key( +async def get_api_key( app: web.Application, *, - user_id: UserID, product_name: ProductName, - display_name: str, - expiration: dt.timedelta | None = None, + user_id: UserID, + api_key_id: str, ) -> ApiKey: - - key, secret = _generate_api_key_and_secret(display_name) - - api_key: ApiKey = await _repository.get_or_create_api_key( + api_key: ApiKey | None = await _repository.get_api_key( app, + api_key_id=api_key_id, user_id=user_id, product_name=product_name, - display_name=display_name, - expiration=expiration, - api_key=key, - api_secret=secret, ) + if api_key is not None: + return api_key - return api_key + raise ApiKeyNotFoundError(api_key_id=api_key_id) -async def delete_api_key( +async def list_api_keys( app: web.Application, *, - api_key_id: str, - user_id: UserID, product_name: ProductName, -) -> None: - await _repository.delete_api_key( - app, - api_key_id=api_key_id, - user_id=user_id, - product_name=product_name, + user_id: UserID, +) -> list[ApiKey]: + api_keys: list[ApiKey] = await _repository.list_api_keys( + app, user_id=user_id, product_name=product_name ) + return api_keys async def prune_expired_api_keys(app: web.Application) -> list[str]: diff --git a/services/web/server/src/simcore_service_webserver/api_keys/plugin.py b/services/web/server/src/simcore_service_webserver/api_keys/plugin.py index 11aeabf1e330..9ba6062c5e46 100644 --- a/services/web/server/src/simcore_service_webserver/api_keys/plugin.py +++ b/services/web/server/src/simcore_service_webserver/api_keys/plugin.py @@ -8,7 +8,7 @@ from ..products.plugin import setup_products from ..rabbitmq import setup_rabbitmq from ..rest.plugin import setup_rest -from . import _controller_rest, _controller_rpc +from ._controller import rest, rpc _logger = logging.getLogger(__name__) @@ -26,9 +26,9 @@ def setup_api_keys(app: web.Application): # http api setup_rest(app) - app.router.add_routes(_controller_rest.routes) + app.router.add_routes(rest.routes) # rpc api setup_rabbitmq(app) if app[APP_SETTINGS_KEY].WEBSERVER_RABBITMQ: - app.on_startup.append(_controller_rpc.register_rpc_routes_on_startup) + app.on_startup.append(rpc.register_rpc_routes_on_startup) diff --git a/services/web/server/tests/unit/with_dbs/01/test_api_keys.py b/services/web/server/tests/unit/with_dbs/01/test_api_keys.py index cfd7b9f154a6..a486dd856033 100644 --- a/services/web/server/tests/unit/with_dbs/01/test_api_keys.py +++ b/services/web/server/tests/unit/with_dbs/01/test_api_keys.py @@ -18,9 +18,9 @@ from pytest_simcore.helpers.assert_checks import assert_status from pytest_simcore.helpers.monkeypatch_envs import setenvs_from_dict from pytest_simcore.helpers.typing_env import EnvVarsDict -from pytest_simcore.helpers.webserver_login import NewUser, UserInfoDict +from pytest_simcore.helpers.webserver_login import UserInfoDict from servicelib.aiohttp import status -from simcore_service_webserver.api_keys import _repository, _service, api_keys_service +from simcore_service_webserver.api_keys import _repository, api_keys_service from simcore_service_webserver.api_keys.models import ApiKey from simcore_service_webserver.application_settings import ( ApplicationSettings, @@ -201,34 +201,6 @@ async def test_create_api_key_with_expiration( assert not data -async def test_get_or_create_api_key( - disabled_setup_garbage_collector: MockType, - client: TestClient, -): - async with NewUser( - app=client.app, - ) as user: - - assert client.app - - options = { - "user_id": user["id"], - "product_name": "osparc", - "display_name": "foo", - } - - # create once - created = await _service.get_or_create_api_key(client.app, **options) - assert created.display_name == "foo" - assert created.api_key != created.api_secret - - # idempotent - for _ in range(3): - assert ( - await _service.get_or_create_api_key(client.app, **options) == created - ) - - @pytest.mark.parametrize( "user_role,expected", _get_user_access_parametrizations(status.HTTP_404_NOT_FOUND), diff --git a/services/web/server/tests/unit/with_dbs/01/test_api_keys_rpc.py b/services/web/server/tests/unit/with_dbs/01/test_api_keys_rpc.py index 3053df387976..fca3bf0177ac 100644 --- a/services/web/server/tests/unit/with_dbs/01/test_api_keys_rpc.py +++ b/services/web/server/tests/unit/with_dbs/01/test_api_keys_rpc.py @@ -8,6 +8,7 @@ import pytest from aiohttp.test_utils import TestServer from faker import Faker +from models_library.basic_types import IDStr from models_library.products import ProductName from models_library.rpc.webserver.auth.api_keys import ApiKeyCreate from pytest_mock import MockerFixture @@ -17,7 +18,7 @@ from servicelib.rabbitmq import RabbitMQRPCClient from servicelib.rabbitmq.rpc_interfaces.webserver.auth.api_keys import ( create_api_key, - delete_api_key, + delete_api_key_by_key, get_api_key, ) from settings_library.rabbit import RabbitSettings @@ -114,7 +115,7 @@ async def test_get_api_key( rpc_client, user_id=logged_user["id"], product_name=osparc_product_name, - api_key_id=api_key.id, + api_key_id=IDStr(api_key.id), ) assert result.id == api_key.id @@ -147,14 +148,16 @@ async def test_api_keys_workflow( assert queried_api_key.display_name == key_name assert created_api_key.id == queried_api_key.id + assert created_api_key.api_key + assert created_api_key.api_key == queried_api_key.api_key assert created_api_key.display_name == queried_api_key.display_name # remove the key - await delete_api_key( + await delete_api_key_by_key( rpc_client, user_id=logged_user["id"], product_name=osparc_product_name, - api_key_id=created_api_key.id, + api_key=created_api_key.api_key, ) with pytest.raises(ApiKeyNotFoundError):