🔒️ Enhance security of API keys 🗃️ (#7085)

Co-authored-by: Copilot <[email protected]>

Author: giancarloromeo

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,
 )
 

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)]

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

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")

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(),

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()

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

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

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:

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):