diff --git a/api/controllers/console/apikey.py b/api/controllers/console/apikey.py index dcea303d7cc..2bd050464f9 100644 --- a/api/controllers/console/apikey.py +++ b/api/controllers/console/apikey.py @@ -1,11 +1,13 @@ +from collections.abc import Iterable from datetime import datetime +from typing import override from uuid import UUID import flask_restx from flask_restx import Resource from flask_restx._http import HTTPStatus from pydantic import field_validator -from sqlalchemy import delete, func, select +from sqlalchemy import delete, func, or_, select from sqlalchemy.orm import sessionmaker from werkzeug.exceptions import Forbidden @@ -38,6 +40,9 @@ class ApiKeyItem(ResponseModel): id: str type: str token: str + # Set only for dataset keys bound to a single knowledge base; None means the + # key is workspace-scoped (app keys are always None). + dataset_id: str | None = None last_used_at: int | None = None created_at: int | None = None @@ -54,6 +59,28 @@ class ApiKeyList(ResponseModel): register_response_schema_models(console_ns, ApiKeyItem, ApiKeyList) +def mask_api_token(token: str) -> str: + """Mask a secret token for list responses. + + Reveal-once: the full secret is only returned by the create endpoint. List + endpoints expose just enough (prefix + last 4) to identify a key, never the + full value, so an existing key's secret cannot be retrieved after creation. + """ + if len(token) <= 8: + return "***" + return f"{token[:5]}...{token[-4:]}" + + +def build_masked_api_key_list(api_tokens: Iterable[ApiToken]) -> ApiKeyList: + """Build an ApiKeyList from ORM tokens with their secrets masked.""" + items: list[ApiKeyItem] = [] + for api_token in api_tokens: + item = ApiKeyItem.model_validate(api_token, from_attributes=True) + item.token = mask_api_token(item.token) + items.append(item) + return ApiKeyList(data=items) + + def _get_resource(resource_id, tenant_id, resource_model): with sessionmaker(db.engine).begin() as session: resource = session.execute( @@ -87,7 +114,7 @@ class BaseApiKeyListResource(Resource): ApiToken.type == self.resource_type, getattr(ApiToken, self.resource_id_field) == resource_id ) ).all() - return ApiKeyList.model_validate({"data": keys}, from_attributes=True) + return build_masked_api_key_list(keys) @edit_permission_required def post(self, resource_id: str, current_tenant_id: str) -> tuple[dict[str, object], int]: @@ -224,17 +251,40 @@ class AppApiKeyResource(BaseApiKeyResource): @console_ns.route("/datasets//api-keys") class DatasetApiKeyListResource(BaseApiKeyListResource): + """Per-dataset API keys: keys created here are bound to a single dataset. + + Binding is stored in ``ApiToken.dataset_id`` and enforced by + ``validate_dataset_token`` (controllers/service_api/wraps.py). Workspace-scoped + keys (NULL ``dataset_id``) are managed by ``DatasetApiKeyApi`` in + controllers/console/datasets/datasets.py. + """ + @console_ns.doc("get_dataset_api_keys") - @console_ns.doc(description="Get all API keys for a dataset") + @console_ns.doc(description="Get all API keys that can access a dataset") @console_ns.doc(params={"resource_id": "Dataset ID"}) @console_ns.response(200, "API keys retrieved successfully", console_ns.models[ApiKeyList.__name__]) @with_current_tenant_id def get(self, current_tenant_id: str, resource_id: UUID) -> dict[str, object]: - """Get all API keys for a dataset""" + """Get all API keys that can access a dataset""" return dump_response(ApiKeyList, self._get_api_key_list(str(resource_id), current_tenant_id)) + @override + def _get_api_key_list(self, resource_id: str, current_tenant_id: str) -> ApiKeyList: + # Unlike the app list, this returns every key that can reach the dataset: + # keys bound to it plus the tenant's workspace-scoped (NULL dataset_id) keys, + # so the dataset page shows the full access picture rather than a subset. + _get_resource(resource_id, current_tenant_id, self.resource_model) + keys = db.session.scalars( + select(ApiToken).where( + ApiToken.type == self.resource_type, + ApiToken.tenant_id == current_tenant_id, + or_(ApiToken.dataset_id == resource_id, ApiToken.dataset_id.is_(None)), + ) + ).all() + return build_masked_api_key_list(keys) + @console_ns.doc("create_dataset_api_key") - @console_ns.doc(description="Create a new API key for a dataset") + @console_ns.doc(description="Create a new API key bound to a single dataset") @console_ns.doc(params={"resource_id": "Dataset ID"}) @console_ns.response(201, "API key created successfully", console_ns.models[ApiKeyItem.__name__]) @console_ns.response(400, "Maximum keys exceeded") @@ -242,13 +292,15 @@ class DatasetApiKeyListResource(BaseApiKeyListResource): @edit_permission_required @rbac_permission_required(RBACResourceScope.DATASET, RBACPermission.DATASET_API_KEY_MANAGE) def post(self, current_tenant_id: str, resource_id: UUID) -> tuple[dict[str, object], int]: - """Create a new API key for a dataset""" + """Create a new API key bound to a single dataset""" return dump_response(ApiKeyItem, self._create_api_key(str(resource_id), current_tenant_id)), 201 resource_type = ApiTokenType.DATASET resource_model = Dataset resource_id_field = "dataset_id" - token_prefix = "ds-" + # Same prefix as workspace-scoped dataset keys (datasets.py); scope is carried + # by the dataset_id column, not the token text. + token_prefix = "dataset-" @console_ns.route("/datasets//api-keys/") diff --git a/api/controllers/console/datasets/datasets.py b/api/controllers/console/datasets/datasets.py index 55bc85483d5..22259611c78 100644 --- a/api/controllers/console/datasets/datasets.py +++ b/api/controllers/console/datasets/datasets.py @@ -13,7 +13,7 @@ from configs import dify_config from controllers.common.fields import ApiBaseUrlResponse, SimpleResultResponse, UsageCheckResponse from controllers.common.schema import query_params_from_model, register_response_schema_models, register_schema_models from controllers.console import console_ns -from controllers.console.apikey import ApiKeyItem, ApiKeyList +from controllers.console.apikey import ApiKeyItem, ApiKeyList, build_masked_api_key_list from controllers.console.app.error import ProviderNotInitializeError from controllers.console.datasets.error import DatasetInUseError, DatasetNameDuplicateError, IndexingEstimateError from controllers.console.wraps import ( @@ -1007,7 +1007,7 @@ class DatasetApiKeyApi(Resource): keys = db.session.scalars( select(ApiToken).where(ApiToken.type == self.resource_type, ApiToken.tenant_id == current_tenant_id) ).all() - return ApiKeyList.model_validate({"data": keys}, from_attributes=True).model_dump(mode="json") + return build_masked_api_key_list(keys).model_dump(mode="json") @console_ns.response(200, "API key created successfully", console_ns.models[ApiKeyItem.__name__]) @console_ns.response(400, "Maximum keys exceeded") diff --git a/api/controllers/service_api/wraps.py b/api/controllers/service_api/wraps.py index 32e95b481f8..1be5c4a3cbb 100644 --- a/api/controllers/service_api/wraps.py +++ b/api/controllers/service_api/wraps.py @@ -305,6 +305,14 @@ def validate_dataset_token[R](view: Callable[..., R]) -> Callable[..., R]: except Exception: logger.exception("Failed to parse dataset_id from positional args") + # A dataset-bound token (non-NULL dataset_id) may only call endpoints that + # carry its own dataset id; endpoints without one (e.g. list/create datasets) + # are rejected outright. Workspace-scoped tokens (dataset_id IS NULL) keep + # tenant-wide access, which preserves behavior for all keys created before + # per-dataset scoping existed. + if api_token.dataset_id and (not dataset_id or str(dataset_id) != str(api_token.dataset_id)): + raise Forbidden("The API key is not authorized to access this knowledge base.") + if dataset_id: dataset_id = str(dataset_id) dataset = db.session.scalar( diff --git a/api/migrations/versions/2026_06_17_0900-e4f8a2c61d35_add_dataset_id_to_api_tokens.py b/api/migrations/versions/2026_06_17_0900-e4f8a2c61d35_add_dataset_id_to_api_tokens.py new file mode 100644 index 00000000000..844a8d6c932 --- /dev/null +++ b/api/migrations/versions/2026_06_17_0900-e4f8a2c61d35_add_dataset_id_to_api_tokens.py @@ -0,0 +1,40 @@ +"""add dataset_id to api_tokens + +Revision ID: e4f8a2c61d35 +Revises: d9e8f7a6b5c4 +Create Date: 2026-06-17 09:00:00.000000 + +Reintroduces the nullable `dataset_id` column on `api_tokens` (it was dropped in +2e9819ca5b28 when dataset keys became tenant-scoped) to support API keys bound to +a single dataset/knowledge base: + + NULL — workspace-scoped key (default; behavior of every pre-existing key). + — key may only access the bound dataset; enforced in + controllers/service_api/wraps.py::validate_dataset_token. + +No backfill is needed: NULL is the correct value for all existing rows. The column +is nullable with no default, so this is a metadata-only change on PostgreSQL. +""" + +import sqlalchemy as sa +from alembic import op + +import models as models + +# revision identifiers, used by Alembic. +revision = "e4f8a2c61d35" +down_revision = "d9e8f7a6b5c4" +branch_labels = None +depends_on = None + + +def upgrade(): + with op.batch_alter_table("api_tokens", schema=None) as batch_op: + batch_op.add_column(sa.Column("dataset_id", models.types.StringUUID(), nullable=True)) + batch_op.create_index("api_token_dataset_id_idx", ["dataset_id", "type"], unique=False) + + +def downgrade(): + with op.batch_alter_table("api_tokens", schema=None) as batch_op: + batch_op.drop_index("api_token_dataset_id_idx") + batch_op.drop_column("dataset_id") diff --git a/api/models/model.py b/api/models/model.py index 38d67004de4..82034010092 100644 --- a/api/models/model.py +++ b/api/models/model.py @@ -2226,18 +2226,36 @@ class Site(Base): return dify_config.APP_WEB_URL or request.url_root.rstrip("/") -class ApiToken(Base): # bug: this uses setattr so idk the field. +class ApiToken(Base): + """API token for the service API. + + Scoping rules: + - ``type`` = "app": ``app_id`` points at the app the key serves. + - ``type`` = "dataset": ``tenant_id`` is always set. ``dataset_id`` is NULL for + workspace-scoped keys (full access to every dataset in the tenant — the default + and the only behavior before the column was reintroduced) or set to bind the key + to a single dataset (least-privilege keys created from a knowledge base page). + Enforcement lives in ``validate_dataset_token`` (controllers/service_api/wraps.py); + cached copies must mirror this field (services/api_token_service.CachedApiToken). + + Note: controllers/console/apikey.py assigns the *_id columns via ``setattr`` keyed + on ``resource_id_field``, so renaming ``app_id``/``dataset_id`` requires updating + those controllers too. + """ + __tablename__ = "api_tokens" __table_args__ = ( sa.PrimaryKeyConstraint("id", name="api_token_pkey"), sa.Index("api_token_app_id_type_idx", "app_id", "type"), sa.Index("api_token_token_idx", "token", "type"), sa.Index("api_token_tenant_idx", "tenant_id", "type"), + sa.Index("api_token_dataset_id_idx", "dataset_id", "type"), ) id = mapped_column(StringUUID, default=lambda: str(uuid4())) app_id = mapped_column(StringUUID, nullable=True) tenant_id = mapped_column(StringUUID, nullable=True) + dataset_id: Mapped[str | None] = mapped_column(StringUUID, nullable=True) type: Mapped[ApiTokenType] = mapped_column(EnumText(ApiTokenType, length=16), nullable=False) token: Mapped[str] = mapped_column(String(255), nullable=False) last_used_at = mapped_column(sa.DateTime, nullable=True) diff --git a/api/openapi/markdown/console-openapi.md b/api/openapi/markdown/console-openapi.md index b3a0b8a6a71..acb54bf05ca 100644 --- a/api/openapi/markdown/console-openapi.md +++ b/api/openapi/markdown/console-openapi.md @@ -6060,7 +6060,7 @@ Check if dataset is in use | 200 | Dataset use status retrieved successfully | **application/json**: [UsageCheckResponse](#usagecheckresponse)
| ### [GET] /datasets/{resource_id}/api-keys -**Get all API keys for a dataset** +**Get all API keys that can access a dataset** #### Parameters @@ -6075,7 +6075,7 @@ Check if dataset is in use | 200 | API keys retrieved successfully | **application/json**: [ApiKeyList](#apikeylist)
| ### [POST] /datasets/{resource_id}/api-keys -**Create a new API key for a dataset** +**Create a new API key bound to a single dataset** #### Parameters @@ -13647,6 +13647,7 @@ Soft lifecycle state for Agent records. | Name | Type | Description | Required | | ---- | ---- | ----------- | -------- | | created_at | integer | | No | +| dataset_id | string | | No | | id | string | | Yes | | last_used_at | integer | | No | | token | string | | Yes | diff --git a/api/services/api_token_service.py b/api/services/api_token_service.py index e9d887195a7..bf44f3ab6b7 100644 --- a/api/services/api_token_service.py +++ b/api/services/api_token_service.py @@ -38,6 +38,9 @@ class CachedApiToken(BaseModel): id: str app_id: str | None tenant_id: str | None + # Defaults to None so cache entries written before this field existed keep + # deserializing; a validation failure here would 401 live tokens until TTL expiry. + dataset_id: str | None = None type: str token: str last_used_at: datetime | None @@ -95,6 +98,7 @@ class ApiTokenCache: id=str(api_token.id), app_id=str(api_token.app_id) if api_token.app_id else None, tenant_id=str(api_token.tenant_id) if api_token.tenant_id else None, + dataset_id=str(api_token.dataset_id) if api_token.dataset_id else None, type=api_token.type, token=api_token.token, last_used_at=api_token.last_used_at, diff --git a/api/tests/test_containers_integration_tests/controllers/console/test_apikey.py b/api/tests/test_containers_integration_tests/controllers/console/test_apikey.py index 8559501c898..c289ced786e 100644 --- a/api/tests/test_containers_integration_tests/controllers/console/test_apikey.py +++ b/api/tests/test_containers_integration_tests/controllers/console/test_apikey.py @@ -3,6 +3,7 @@ from __future__ import annotations from unittest.mock import MagicMock, patch +from uuid import uuid4 import pytest from flask import Flask @@ -12,7 +13,8 @@ from sqlalchemy.orm import Session from models import Account from models.account import AccountStatus, TenantAccountRole -from models.enums import ApiTokenType +from models.dataset import Dataset +from models.enums import ApiTokenType, DataSourceType from models.model import ApiToken, App, AppMode from tests.test_containers_integration_tests.controllers.console.helpers import ( authenticate_console_client, @@ -33,6 +35,28 @@ def setup_app( return test_client_with_containers, headers, app +@pytest.fixture +def setup_dataset( + db_session_with_containers: Session, + test_client_with_containers: FlaskClient, +) -> tuple[FlaskClient, dict[str, str], Dataset]: + """Create an authenticated client with a dataset for per-dataset API key tests.""" + account, tenant = create_console_account_and_tenant(db_session_with_containers) + dataset = Dataset( + tenant_id=tenant.id, + name=f"API Key Dataset {uuid4()}", + description="Dataset for API key scoping tests", + data_source_type=DataSourceType.UPLOAD_FILE, + created_by=account.id, + permission="only_me", + provider="vendor", + ) + db_session_with_containers.add(dataset) + db_session_with_containers.commit() + headers = authenticate_console_client(test_client_with_containers, account) + return test_client_with_containers, headers, dataset + + @pytest.fixture(autouse=True) def cleanup_api_tokens(db_session_with_containers: Session): """Remove API tokens created during each test.""" @@ -184,3 +208,117 @@ class TestAppApiKeyResource: ): with pytest.raises(Forbidden): BaseApiKeyResource.delete(resource, "rid", "kid", "tenant-id", non_admin) + + +class TestDatasetApiKeyListResource: + """Tests for GET/POST /datasets//api-keys (dataset-bound keys).""" + + def test_create_dataset_bound_key( + self, + setup_dataset: tuple[FlaskClient, dict[str, str], Dataset], + db_session_with_containers: Session, + ) -> None: + client, headers, dataset = setup_dataset + + resp = client.post(f"/console/api/datasets/{dataset.id}/api-keys", headers=headers) + + assert resp.status_code == 201 + assert resp.json is not None + assert resp.json["token"].startswith("dataset-") + assert resp.json["dataset_id"] == dataset.id + api_token = db_session_with_containers.scalar(select(ApiToken).where(ApiToken.id == resp.json["id"])) + assert api_token is not None + assert api_token.dataset_id == dataset.id + assert api_token.tenant_id == dataset.tenant_id + assert api_token.type == ApiTokenType.DATASET + + def test_list_includes_bound_and_workspace_scoped_keys( + self, + setup_dataset: tuple[FlaskClient, dict[str, str], Dataset], + db_session_with_containers: Session, + ) -> None: + client, headers, dataset = setup_dataset + + # A bound key via the per-dataset route and a workspace key via the tenant route. + bound_resp = client.post(f"/console/api/datasets/{dataset.id}/api-keys", headers=headers) + assert bound_resp.status_code == 201 + workspace_resp = client.post("/console/api/datasets/api-keys", headers=headers) + assert workspace_resp.status_code == 200 + + resp = client.get(f"/console/api/datasets/{dataset.id}/api-keys", headers=headers) + + assert resp.status_code == 200 + assert resp.json is not None + scopes = {item["id"]: item["dataset_id"] for item in resp.json["data"]} + assert bound_resp.json is not None + assert workspace_resp.json is not None + assert scopes[bound_resp.json["id"]] == dataset.id + assert scopes[workspace_resp.json["id"]] is None + + def test_list_excludes_keys_bound_to_other_datasets( + self, + setup_dataset: tuple[FlaskClient, dict[str, str], Dataset], + db_session_with_containers: Session, + ) -> None: + client, headers, dataset = setup_dataset + # Capture ids up front: the commit below expires these instances and the + # subsequent request teardown detaches them, so reading dataset.id afterwards + # would raise DetachedInstanceError. + dataset_id = dataset.id + other_dataset = Dataset( + tenant_id=dataset.tenant_id, + name=f"Other Dataset {uuid4()}", + description="Second dataset", + data_source_type=DataSourceType.UPLOAD_FILE, + created_by=dataset.created_by, + permission="only_me", + provider="vendor", + ) + db_session_with_containers.add(other_dataset) + db_session_with_containers.commit() + other_dataset_id = other_dataset.id + + other_resp = client.post(f"/console/api/datasets/{other_dataset_id}/api-keys", headers=headers) + assert other_resp.status_code == 201 + + resp = client.get(f"/console/api/datasets/{dataset_id}/api-keys", headers=headers) + + assert resp.status_code == 200 + assert resp.json is not None + assert other_resp.json is not None + assert other_resp.json["id"] not in {item["id"] for item in resp.json["data"]} + + def test_workspace_route_creates_unbound_key( + self, + setup_dataset: tuple[FlaskClient, dict[str, str], Dataset], + db_session_with_containers: Session, + ) -> None: + """The pre-existing workspace route must keep creating NULL-scoped keys.""" + client, headers, _ = setup_dataset + + resp = client.post("/console/api/datasets/api-keys", headers=headers) + + assert resp.status_code == 200 + assert resp.json is not None + api_token = db_session_with_containers.scalar(select(ApiToken).where(ApiToken.id == resp.json["id"])) + assert api_token is not None + assert api_token.dataset_id is None + + +class TestDatasetApiKeyResource: + """Tests for DELETE /datasets//api-keys/.""" + + def test_delete_bound_key( + self, + setup_dataset: tuple[FlaskClient, dict[str, str], Dataset], + ) -> None: + client, headers, dataset = setup_dataset + create_resp = client.post(f"/console/api/datasets/{dataset.id}/api-keys", headers=headers) + assert create_resp.json is not None + + resp = client.delete( + f"/console/api/datasets/{dataset.id}/api-keys/{create_resp.json['id']}", + headers=headers, + ) + + assert resp.status_code == 204 diff --git a/api/tests/unit_tests/controllers/console/datasets/test_datasets.py b/api/tests/unit_tests/controllers/console/datasets/test_datasets.py index 76a09558987..a2b1744cafc 100644 --- a/api/tests/unit_tests/controllers/console/datasets/test_datasets.py +++ b/api/tests/unit_tests/controllers/console/datasets/test_datasets.py @@ -1758,13 +1758,15 @@ class TestDatasetApiKeyApi: mock_key_1 = MagicMock(spec=ApiToken) mock_key_1.id = "key-1" mock_key_1.type = "dataset" - mock_key_1.token = "ds-abc" + mock_key_1.dataset_id = None + mock_key_1.token = "dataset-aaaa1111bbbb" mock_key_1.last_used_at = None mock_key_1.created_at = None mock_key_2 = MagicMock(spec=ApiToken) mock_key_2.id = "key-2" mock_key_2.type = "dataset" - mock_key_2.token = "ds-def" + mock_key_2.dataset_id = None + mock_key_2.token = "dataset-cccc2222dddd" mock_key_2.last_used_at = None mock_key_2.created_at = None @@ -1779,10 +1781,11 @@ class TestDatasetApiKeyApi: assert "data" in response assert len(response["data"]) == 2 + # reveal-once: the list returns masked tokens, never the full secret assert response["data"][0]["id"] == "key-1" - assert response["data"][0]["token"] == "ds-abc" + assert response["data"][0]["token"] == "datas...bbbb" assert response["data"][1]["id"] == "key-2" - assert response["data"][1]["token"] == "ds-def" + assert response["data"][1]["token"] == "datas...dddd" def test_post_create_api_key_success(self, app: Flask): api = DatasetApiKeyApi() @@ -1790,6 +1793,7 @@ class TestDatasetApiKeyApi: mock_token = MagicMock() mock_token.id = "new-key-id" + mock_token.dataset_id = None mock_token.last_used_at = None mock_token.created_at = datetime.datetime(2024, 1, 1, 0, 0, 0, tzinfo=datetime.UTC) diff --git a/api/tests/unit_tests/controllers/console/test_apikey.py b/api/tests/unit_tests/controllers/console/test_apikey.py index 1517ff5ed8e..06fa7a62d70 100644 --- a/api/tests/unit_tests/controllers/console/test_apikey.py +++ b/api/tests/unit_tests/controllers/console/test_apikey.py @@ -9,9 +9,16 @@ from unittest.mock import patch import pytest from werkzeug.exceptions import Forbidden -from controllers.console.apikey import BaseApiKeyListResource, BaseApiKeyResource +from controllers.console.apikey import ( + BaseApiKeyListResource, + BaseApiKeyResource, + DatasetApiKeyListResource, + build_masked_api_key_list, + mask_api_token, +) from models import Account from models.account import AccountStatus, TenantAccountRole +from models.dataset import Dataset from models.enums import ApiTokenType from models.model import ApiToken, App @@ -44,12 +51,37 @@ def _make_account(role: TenantAccountRole) -> Account: return account +def test_mask_api_token_reveals_only_a_fragment() -> None: + # full secret must never be reproducible from the masked value + masked = mask_api_token("dataset-mqxAkpML14jRmgsb6Z7DBnVq") + assert masked == "datas...BnVq" + assert "mqxAkpML" not in masked + # very short tokens are fully hidden + assert mask_api_token("short") == "***" + + +def test_build_masked_api_key_list_masks_every_token() -> None: + keys = [ + SimpleNamespace( + id="key-1", + type=ApiTokenType.DATASET, + token="dataset-aaaabbbbccccdddd", + dataset_id="ds-1", + last_used_at=None, + created_at=None, + ), + ] + result = build_masked_api_key_list(keys) + assert result.data[0].token == "datas...dddd" + assert result.data[0].dataset_id == "ds-1" + + def test_list_api_keys_uses_injected_tenant_id() -> None: resource = _make_list_resource() api_key = SimpleNamespace( id="key-1", type=ApiTokenType.APP, - token="app-token", + token="app-1234567890abcdef", last_used_at=None, created_at=None, ) @@ -67,8 +99,10 @@ def test_list_api_keys_uses_injected_tenant_id() -> None: "data": [ { "id": "key-1", + # reveal-once: the list never returns the full secret, only a masked fragment "type": "app", - "token": "app-token", + "token": "app-1...cdef", + "dataset_id": None, "last_used_at": None, "created_at": None, } @@ -106,6 +140,46 @@ def test_create_api_key_uses_injected_tenant_id() -> None: db_mock.session.commit.assert_called_once() +def test_create_dataset_api_key_binds_dataset_id() -> None: + """Creating a key on the per-dataset route must bind it to that dataset (ApiToken.dataset_id).""" + resource = DatasetApiKeyListResource() + + def add_api_token(api_token: ApiToken) -> None: + api_token.id = "key-1" + + with ( + patch("controllers.console.apikey._get_resource") as get_resource, + patch("controllers.console.apikey.db") as db_mock, + patch("controllers.console.apikey.ApiToken.generate_api_key", return_value="dataset-generated-token"), + ): + db_mock.session.scalar.return_value = 0 + db_mock.session.add.side_effect = add_api_token + + api_token = resource._create_api_key("dataset-1", "tenant-1") + + get_resource.assert_called_once_with("dataset-1", "tenant-1", Dataset) + assert api_token.dataset_id == "dataset-1" + assert api_token.tenant_id == "tenant-1" + assert api_token.type == ApiTokenType.DATASET + db_mock.session.commit.assert_called_once() + + +def test_dataset_api_key_list_includes_workspace_scoped_keys() -> None: + """The per-dataset key list shows everything that can reach the dataset: + keys bound to it plus the tenant's workspace-scoped (NULL dataset_id) keys.""" + resource = DatasetApiKeyListResource() + + with ( + patch("controllers.console.apikey._get_resource"), + patch("controllers.console.apikey.db") as db_mock, + ): + db_mock.session.scalars.return_value.all.return_value = [] + resource._get_api_key_list("dataset-1", "tenant-1") + + query_sql = str(db_mock.session.scalars.call_args.args[0]) + assert "dataset_id IS NULL" in query_sql + + def test_delete_api_key_rejects_non_admin_account() -> None: resource = _make_key_resource() diff --git a/api/tests/unit_tests/controllers/service_api/test_wraps.py b/api/tests/unit_tests/controllers/service_api/test_wraps.py index 0b5c5d95b69..92baa03c8e0 100644 --- a/api/tests/unit_tests/controllers/service_api/test_wraps.py +++ b/api/tests/unit_tests/controllers/service_api/test_wraps.py @@ -516,6 +516,7 @@ class TestValidateDatasetToken: tenant_id = str(uuid.uuid4()) mock_api_token = Mock() mock_api_token.tenant_id = tenant_id + mock_api_token.dataset_id = None mock_validate_token.return_value = mock_api_token mock_tenant = Mock() @@ -554,6 +555,7 @@ class TestValidateDatasetToken: # Arrange mock_api_token = Mock() mock_api_token.tenant_id = str(uuid.uuid4()) + mock_api_token.dataset_id = None mock_validate_token.return_value = mock_api_token mock_db.session.scalar.return_value = None @@ -568,6 +570,134 @@ class TestValidateDatasetToken: protected_view(dataset_id=str(uuid.uuid4())) assert "Dataset not found" in str(exc_info.value) + def _arrange_tenant_owner(self, mock_db, tenant_id: str) -> None: + """Stub the tenant-owner resolution shared by all success-path tests.""" + mock_tenant = Mock() + mock_tenant.id = tenant_id + mock_tenant.status = TenantStatus.NORMAL + + mock_ta = Mock() + mock_ta.account_id = str(uuid.uuid4()) + + mock_account = Mock() + mock_account.id = mock_ta.account_id + mock_account.current_tenant = mock_tenant + + setup_mock_dataset_owner_execute_result(mock_db, mock_tenant, mock_ta) + mock_db.session.get.return_value = mock_account + + @patch("controllers.service_api.wraps.user_logged_in") + @patch("controllers.service_api.wraps.db") + @patch("controllers.service_api.wraps.validate_and_get_api_token") + @patch("controllers.service_api.wraps.current_app") + def test_workspace_scoped_token_can_access_any_dataset( + self, mock_current_app, mock_validate_token, mock_db, mock_user_logged_in, app: Flask + ): + """A token without dataset binding (NULL dataset_id) keeps tenant-wide access.""" + # Arrange + _configure_current_app_mock(mock_current_app) + + tenant_id = str(uuid.uuid4()) + dataset_id = str(uuid.uuid4()) + mock_api_token = Mock() + mock_api_token.tenant_id = tenant_id + mock_api_token.dataset_id = None + mock_validate_token.return_value = mock_api_token + + mock_dataset = Mock() + mock_dataset.id = dataset_id + mock_dataset.enable_api = True + mock_db.session.scalar.return_value = mock_dataset + + self._arrange_tenant_owner(mock_db, tenant_id) + + @validate_dataset_token + def protected_view(tenant_id, dataset_id=None): + return {"success": True} + + # Act + with app.test_request_context("/", method="GET", headers={"Authorization": "Bearer test_token"}): + result = protected_view(dataset_id=dataset_id) + + # Assert + assert result["success"] is True + + @patch("controllers.service_api.wraps.user_logged_in") + @patch("controllers.service_api.wraps.db") + @patch("controllers.service_api.wraps.validate_and_get_api_token") + @patch("controllers.service_api.wraps.current_app") + def test_dataset_bound_token_allows_its_own_dataset( + self, mock_current_app, mock_validate_token, mock_db, mock_user_logged_in, app: Flask + ): + """A dataset-bound token can access the dataset it is bound to.""" + # Arrange + _configure_current_app_mock(mock_current_app) + + tenant_id = str(uuid.uuid4()) + dataset_id = str(uuid.uuid4()) + mock_api_token = Mock() + mock_api_token.tenant_id = tenant_id + mock_api_token.dataset_id = dataset_id + mock_validate_token.return_value = mock_api_token + + mock_dataset = Mock() + mock_dataset.id = dataset_id + mock_dataset.enable_api = True + mock_db.session.scalar.return_value = mock_dataset + + self._arrange_tenant_owner(mock_db, tenant_id) + + @validate_dataset_token + def protected_view(tenant_id, dataset_id=None): + return {"success": True} + + # Act + with app.test_request_context("/", method="GET", headers={"Authorization": "Bearer test_token"}): + result = protected_view(dataset_id=dataset_id) + + # Assert + assert result["success"] is True + + @patch("controllers.service_api.wraps.db") + @patch("controllers.service_api.wraps.validate_and_get_api_token") + def test_dataset_bound_token_rejects_other_dataset(self, mock_validate_token, mock_db, app: Flask): + """A dataset-bound token gets Forbidden for any other dataset in the tenant.""" + # Arrange + mock_api_token = Mock() + mock_api_token.tenant_id = str(uuid.uuid4()) + mock_api_token.dataset_id = str(uuid.uuid4()) + mock_validate_token.return_value = mock_api_token + + @validate_dataset_token + def protected_view(tenant_id, dataset_id=None): + return {"success": True} + + # Act & Assert + with app.test_request_context("/", method="GET"): + with pytest.raises(Forbidden) as exc_info: + protected_view(dataset_id=str(uuid.uuid4())) + assert "not authorized" in str(exc_info.value) + + @patch("controllers.service_api.wraps.db") + @patch("controllers.service_api.wraps.validate_and_get_api_token") + def test_dataset_bound_token_rejects_request_without_dataset_id(self, mock_validate_token, mock_db, app: Flask): + """A dataset-bound token cannot call endpoints that carry no dataset id (e.g. list-all).""" + # Arrange + mock_api_token = Mock() + mock_api_token.tenant_id = str(uuid.uuid4()) + mock_api_token.dataset_id = str(uuid.uuid4()) + mock_validate_token.return_value = mock_api_token + + @validate_dataset_token + def protected_view(tenant_id): + return {"success": True} + + # Act & Assert + with app.test_request_context("/", method="GET"): + with pytest.raises(Forbidden) as exc_info: + protected_view() + assert "not authorized" in str(exc_info.value) + class TestFetchUserArg: """Test suite for FetchUserArg model""" diff --git a/api/tests/unit_tests/libs/test_api_token_cache.py b/api/tests/unit_tests/libs/test_api_token_cache.py index fa4c5e77a71..ba2e2bb454a 100644 --- a/api/tests/unit_tests/libs/test_api_token_cache.py +++ b/api/tests/unit_tests/libs/test_api_token_cache.py @@ -24,6 +24,7 @@ class TestApiTokenCache: self.mock_token.id = "test-token-id-123" self.mock_token.app_id = "test-app-id-456" self.mock_token.tenant_id = "test-tenant-id-789" + self.mock_token.dataset_id = None self.mock_token.type = "app" self.mock_token.token = "test-token-value-abc" self.mock_token.last_used_at = datetime(2026, 2, 3, 10, 0, 0) @@ -47,6 +48,7 @@ class TestApiTokenCache: assert data["id"] == "test-token-id-123" assert data["app_id"] == "test-app-id-456" assert data["tenant_id"] == "test-tenant-id-789" + assert data["dataset_id"] is None assert data["type"] == "app" assert data["token"] == "test-token-value-abc" assert data["last_used_at"] == "2026-02-03T10:00:00" @@ -58,6 +60,7 @@ class TestApiTokenCache: mock_token.id = "test-id" mock_token.app_id = None mock_token.tenant_id = None + mock_token.dataset_id = None mock_token.type = "dataset" mock_token.token = "test-token" mock_token.last_used_at = None @@ -70,6 +73,24 @@ class TestApiTokenCache: assert data["tenant_id"] is None assert data["last_used_at"] is None + def test_serialize_dataset_bound_token(self): + """Test that a dataset-bound token round-trips its dataset_id through the cache.""" + mock_token = MagicMock() + mock_token.id = "test-id" + mock_token.app_id = None + mock_token.tenant_id = "test-tenant" + mock_token.dataset_id = "test-dataset-id-123" + mock_token.type = "dataset" + mock_token.token = "test-token" + mock_token.last_used_at = None + mock_token.created_at = datetime(2026, 1, 1, 0, 0, 0) + + serialized = ApiTokenCache._serialize_token(mock_token) + deserialized = ApiTokenCache._deserialize_token(serialized) + + assert isinstance(deserialized, CachedApiToken) + assert deserialized.dataset_id == "test-dataset-id-123" + def test_deserialize_token(self): """Test token deserialization.""" cached_data = json.dumps( @@ -94,6 +115,9 @@ class TestApiTokenCache: assert result.token == "test-token" assert result.last_used_at == datetime(2026, 2, 3, 10, 0, 0) assert result.created_at == datetime(2026, 1, 1, 0, 0, 0) + # Cache entries written before the dataset_id field existed must keep + # deserializing (otherwise live tokens 401 until the cache TTL expires). + assert result.dataset_id is None def test_deserialize_null_token(self): """Test deserialization of null token (cached miss).""" @@ -218,6 +242,7 @@ class TestApiTokenCacheIntegration: mock_token.id = "id-123" mock_token.app_id = "app-456" mock_token.tenant_id = "tenant-789" + mock_token.dataset_id = None mock_token.type = "app" mock_token.token = "token-abc" mock_token.last_used_at = datetime(2026, 2, 3, 10, 0, 0) diff --git a/eslint-suppressions.json b/eslint-suppressions.json index e00a1c2a1a3..9301a5d8ddf 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -3290,14 +3290,6 @@ "count": 2 } }, - "web/app/components/develop/secret-key/secret-key-modal.tsx": { - "jsx-a11y/click-events-have-key-events": { - "count": 1 - }, - "jsx-a11y/no-static-element-interactions": { - "count": 1 - } - }, "web/app/components/explore/banner/__tests__/indicator-button.spec.tsx": { "jsx-a11y/click-events-have-key-events": { "count": 1 diff --git a/packages/contracts/generated/api/console/agent/types.gen.ts b/packages/contracts/generated/api/console/agent/types.gen.ts index 43119c4f1f4..3a5dd864c0d 100644 --- a/packages/contracts/generated/api/console/agent/types.gen.ts +++ b/packages/contracts/generated/api/console/agent/types.gen.ts @@ -101,6 +101,7 @@ export type ApiKeyList = { export type ApiKeyItem = { created_at?: number | null + dataset_id?: string | null id: string last_used_at?: number | null token: string diff --git a/packages/contracts/generated/api/console/agent/zod.gen.ts b/packages/contracts/generated/api/console/agent/zod.gen.ts index d7f5681ffc4..ce674c132ce 100644 --- a/packages/contracts/generated/api/console/agent/zod.gen.ts +++ b/packages/contracts/generated/api/console/agent/zod.gen.ts @@ -34,6 +34,7 @@ export const zAgentApiStatusPayload = z.object({ */ export const zApiKeyItem = z.object({ created_at: z.int().nullish(), + dataset_id: z.string().nullish(), id: z.string(), last_used_at: z.int().nullish(), token: z.string(), diff --git a/packages/contracts/generated/api/console/apps/types.gen.ts b/packages/contracts/generated/api/console/apps/types.gen.ts index 9e79518f3cd..cc8268df3b0 100644 --- a/packages/contracts/generated/api/console/apps/types.gen.ts +++ b/packages/contracts/generated/api/console/apps/types.gen.ts @@ -1179,6 +1179,7 @@ export type ApiKeyList = { export type ApiKeyItem = { created_at?: number | null + dataset_id?: string | null id: string last_used_at?: number | null token: string diff --git a/packages/contracts/generated/api/console/apps/zod.gen.ts b/packages/contracts/generated/api/console/apps/zod.gen.ts index 9b86fda0a62..ffbfabb0872 100644 --- a/packages/contracts/generated/api/console/apps/zod.gen.ts +++ b/packages/contracts/generated/api/console/apps/zod.gen.ts @@ -772,6 +772,7 @@ export const zWorkflowRestoreResponse = z.object({ */ export const zApiKeyItem = z.object({ created_at: z.int().nullish(), + dataset_id: z.string().nullish(), id: z.string(), last_used_at: z.int().nullish(), token: z.string(), diff --git a/packages/contracts/generated/api/console/datasets/orpc.gen.ts b/packages/contracts/generated/api/console/datasets/orpc.gen.ts index a8b096168bb..038d4be61bb 100644 --- a/packages/contracts/generated/api/console/datasets/orpc.gen.ts +++ b/packages/contracts/generated/api/console/datasets/orpc.gen.ts @@ -1712,37 +1712,37 @@ export const byApiKeyId2 = { } /** - * Get all API keys for a dataset + * Get all API keys that can access a dataset * - * Get all API keys for a dataset + * Get all API keys that can access a dataset */ export const get35 = oc .route({ - description: 'Get all API keys for a dataset', + description: 'Get all API keys that can access a dataset', inputStructure: 'detailed', method: 'GET', operationId: 'getDatasetsByResourceIdApiKeys', path: '/datasets/{resource_id}/api-keys', - summary: 'Get all API keys for a dataset', + summary: 'Get all API keys that can access a dataset', tags: ['console'], }) .input(z.object({ params: zGetDatasetsByResourceIdApiKeysPath })) .output(zGetDatasetsByResourceIdApiKeysResponse) /** - * Create a new API key for a dataset + * Create a new API key bound to a single dataset * - * Create a new API key for a dataset + * Create a new API key bound to a single dataset */ export const post22 = oc .route({ - description: 'Create a new API key for a dataset', + description: 'Create a new API key bound to a single dataset', inputStructure: 'detailed', method: 'POST', operationId: 'postDatasetsByResourceIdApiKeys', path: '/datasets/{resource_id}/api-keys', successStatus: 201, - summary: 'Create a new API key for a dataset', + summary: 'Create a new API key bound to a single dataset', tags: ['console'], }) .input(z.object({ params: zPostDatasetsByResourceIdApiKeysPath })) diff --git a/packages/contracts/generated/api/console/datasets/types.gen.ts b/packages/contracts/generated/api/console/datasets/types.gen.ts index f904ea7d717..7fc6575ac9d 100644 --- a/packages/contracts/generated/api/console/datasets/types.gen.ts +++ b/packages/contracts/generated/api/console/datasets/types.gen.ts @@ -72,6 +72,7 @@ export type ApiKeyList = { export type ApiKeyItem = { created_at?: number | null + dataset_id?: string | null id: string last_used_at?: number | null token: string diff --git a/packages/contracts/generated/api/console/datasets/zod.gen.ts b/packages/contracts/generated/api/console/datasets/zod.gen.ts index 10ceb11cb5d..edcf515cf2e 100644 --- a/packages/contracts/generated/api/console/datasets/zod.gen.ts +++ b/packages/contracts/generated/api/console/datasets/zod.gen.ts @@ -14,6 +14,7 @@ export const zApiBaseUrlResponse = z.object({ */ export const zApiKeyItem = z.object({ created_at: z.int().nullish(), + dataset_id: z.string().nullish(), id: z.string(), last_used_at: z.int().nullish(), token: z.string(), diff --git a/web/__tests__/develop/api-key-management-flow.test.tsx b/web/__tests__/develop/api-key-management-flow.test.tsx index 0267fdd45e1..c02088455ae 100644 --- a/web/__tests__/develop/api-key-management-flow.test.tsx +++ b/web/__tests__/develop/api-key-management-flow.test.tsx @@ -69,6 +69,7 @@ vi.mock('@/service/use-apps', () => ({ vi.mock('@/service/knowledge/use-dataset', () => ({ useDatasetApiKeys: () => ({ data: null, isLoading: false }), + useDatasetScopedApiKeys: () => ({ data: null, isLoading: false }), useInvalidateDatasetApiKeys: () => vi.fn(), })) diff --git a/web/__tests__/develop/develop-page-flow.test.tsx b/web/__tests__/develop/develop-page-flow.test.tsx index 4c4bcd0f064..f2360aa7455 100644 --- a/web/__tests__/develop/develop-page-flow.test.tsx +++ b/web/__tests__/develop/develop-page-flow.test.tsx @@ -84,6 +84,7 @@ vi.mock('@/service/use-apps', () => ({ vi.mock('@/service/knowledge/use-dataset', () => ({ useDatasetApiKeys: () => ({ data: null, isLoading: false }), + useDatasetScopedApiKeys: () => ({ data: null, isLoading: false }), useInvalidateDatasetApiKeys: () => vi.fn(), })) diff --git a/web/app/components/datasets/extra-info/__tests__/index.spec.tsx b/web/app/components/datasets/extra-info/__tests__/index.spec.tsx index beccaa66600..c41e8d2e70b 100644 --- a/web/app/components/datasets/extra-info/__tests__/index.spec.tsx +++ b/web/app/components/datasets/extra-info/__tests__/index.spec.tsx @@ -1,4 +1,5 @@ import type { DataSet, RelatedApp, RelatedAppResponse } from '@/models/datasets' +import { Popover } from '@langgenius/dify-ui/popover' import { render, screen, waitFor } from '@testing-library/react' import userEvent from '@testing-library/user-event' import { describe, expect, it, vi } from 'vitest' @@ -523,9 +524,12 @@ describe('ApiAccessCard', () => { describe('Rendering', () => { it('should render without crashing', () => { render( - , + + + , ) expect(screen.getByText(/serviceApi\.enabled/i)).toBeInTheDocument() @@ -533,9 +537,12 @@ describe('ApiAccessCard', () => { it('should display enabled status when API is enabled', () => { render( - , + + + , ) expect(screen.getByText(/serviceApi\.enabled/i)).toBeInTheDocument() @@ -543,9 +550,12 @@ describe('ApiAccessCard', () => { it('should display disabled status when API is disabled', () => { render( - , + + + , ) expect(screen.getByText(/serviceApi\.disabled/i)).toBeInTheDocument() @@ -553,9 +563,12 @@ describe('ApiAccessCard', () => { it('should render API Reference link', () => { render( - , + + + , ) expect(screen.getByText(/overview\.apiInfo\.doc/i)).toBeInTheDocument() @@ -563,9 +576,12 @@ describe('ApiAccessCard', () => { it('should render switch component', () => { render( - , + + + , ) expect(screen.getByRole('switch')).toBeInTheDocument() @@ -577,9 +593,12 @@ describe('ApiAccessCard', () => { const user = userEvent.setup() render( - , + + + , ) const switchButton = screen.getByRole('switch') @@ -594,9 +613,12 @@ describe('ApiAccessCard', () => { const user = userEvent.setup() render( - , + + + , ) const switchButton = screen.getByRole('switch') @@ -611,9 +633,12 @@ describe('ApiAccessCard', () => { const user = userEvent.setup() render( - , + + + , ) const switchButton = screen.getByRole('switch') @@ -629,9 +654,12 @@ describe('ApiAccessCard', () => { const user = userEvent.setup() render( - , + + + , ) const switchButton = screen.getByRole('switch') @@ -647,9 +675,12 @@ describe('ApiAccessCard', () => { it('should have correct href for API Reference link', () => { render( - , + + + , ) const apiRefLink = screen.getByText(/overview\.apiInfo\.doc/i).closest('a') @@ -662,9 +693,12 @@ describe('ApiAccessCard', () => { mockDataset.permission_keys = [] render( - , + + + , ) const switchButton = screen.getByRole('switch') @@ -675,9 +709,12 @@ describe('ApiAccessCard', () => { mockDataset.permission_keys = [DatasetACLPermission.Edit] render( - , + + + , ) const switchButton = screen.getByRole('switch') @@ -688,15 +725,21 @@ describe('ApiAccessCard', () => { describe('Memoization', () => { it('should be memoized with React.memo', () => { const { rerender } = render( - , + + + , ) rerender( - , + + + , ) expect(screen.getByText(/serviceApi\.enabled/i)).toBeInTheDocument() @@ -705,15 +748,21 @@ describe('ApiAccessCard', () => { it('should use useCallback for handlers', () => { // Verify handlers are stable by rendering multiple times const { rerender } = render( - , + + + , ) rerender( - , + + + , ) // Component should render without issues with memoized callbacks diff --git a/web/app/components/datasets/extra-info/api-access/__tests__/card.spec.tsx b/web/app/components/datasets/extra-info/api-access/__tests__/card.spec.tsx index fe2ce21986d..275b6ddc9ac 100644 --- a/web/app/components/datasets/extra-info/api-access/__tests__/card.spec.tsx +++ b/web/app/components/datasets/extra-info/api-access/__tests__/card.spec.tsx @@ -1,3 +1,4 @@ +import { Popover } from '@langgenius/dify-ui/popover' import { fireEvent, render, screen, waitFor } from '@testing-library/react' import { beforeEach, describe, expect, it, vi } from 'vitest' import { DatasetACLPermission } from '@/utils/permission' @@ -32,6 +33,16 @@ vi.mock('@/hooks/use-api-access-url', () => ({ useDatasetApiAccessUrl: () => 'https://docs.dify.ai/api-reference/datasets', })) +const onOpenSecretKeyModal = vi.fn() + +// Card renders a PopoverClose, which needs an enclosing Popover root. +const renderCard = (apiEnabled: boolean) => + render( + + + , + ) + describe('Card (API Access)', () => { beforeEach(() => { vi.clearAllMocks() @@ -43,33 +54,33 @@ describe('Card (API Access)', () => { // Rendering: verifies enabled/disabled states render correctly describe('Rendering', () => { it('should render without crashing when api is enabled', () => { - render() + renderCard(true) expect(screen.getByText(/serviceApi\.enabled/)).toBeInTheDocument() }) it('should render without crashing when api is disabled', () => { - render() + renderCard(false) expect(screen.getByText(/serviceApi\.disabled/)).toBeInTheDocument() }) it('should render API access tip text', () => { - render() + renderCard(true) expect(screen.getByText(/appMenus\.apiAccessTip/)).toBeInTheDocument() }) it('should render API reference link', () => { - render() + renderCard(true) const link = screen.getByRole('link') expect(link).toHaveAttribute('href', 'https://docs.dify.ai/api-reference/datasets') }) it('should render API doc text in link', () => { - render() + renderCard(true) expect(screen.getByText(/apiInfo\.doc/)).toBeInTheDocument() }) it('should open API reference link in new tab', () => { - render() + renderCard(true) const link = screen.getByRole('link') expect(link).toHaveAttribute('target', '_blank') expect(link).toHaveAttribute('rel', 'noopener noreferrer') @@ -79,13 +90,13 @@ describe('Card (API Access)', () => { // Props: tests enabled/disabled visual states describe('Props', () => { it('should show green indicator text when enabled', () => { - render() + renderCard(true) const enabledText = screen.getByText(/serviceApi\.enabled/) expect(enabledText).toHaveClass('text-text-success') }) it('should show warning text when disabled', () => { - render() + renderCard(false) const disabledText = screen.getByText(/serviceApi\.disabled/) expect(disabledText).toHaveClass('text-text-warning') }) @@ -95,7 +106,7 @@ describe('Card (API Access)', () => { describe('User Interactions', () => { it('should call enableDatasetServiceApi when toggling on', async () => { mockEnableApi.mockResolvedValue({ result: 'success' }) - render() + renderCard(false) const switchButton = screen.getByRole('switch') fireEvent.click(switchButton) @@ -107,7 +118,7 @@ describe('Card (API Access)', () => { it('should call disableDatasetServiceApi when toggling off', async () => { mockDisableApi.mockResolvedValue({ result: 'success' }) - render() + renderCard(true) const switchButton = screen.getByRole('switch') fireEvent.click(switchButton) @@ -119,7 +130,7 @@ describe('Card (API Access)', () => { it('should call mutateDatasetRes on successful toggle', async () => { mockEnableApi.mockResolvedValue({ result: 'success' }) - render() + renderCard(false) const switchButton = screen.getByRole('switch') fireEvent.click(switchButton) @@ -131,7 +142,7 @@ describe('Card (API Access)', () => { it('should not call mutateDatasetRes when result is not success', async () => { mockEnableApi.mockResolvedValue({ result: 'fail' }) - render() + renderCard(false) const switchButton = screen.getByRole('switch') fireEvent.click(switchButton) @@ -147,7 +158,7 @@ describe('Card (API Access)', () => { describe('Switch State', () => { it('should disable switch when dataset lacks edit ACL permission', () => { mockDatasetPermissionKeys = [] - render() + renderCard(true) const switchButton = screen.getByRole('switch') expect(switchButton).toHaveAttribute('aria-checked', 'true') @@ -156,19 +167,33 @@ describe('Card (API Access)', () => { it('should enable switch when dataset has edit ACL permission', () => { mockDatasetPermissionKeys = [DatasetACLPermission.Edit] - render() + renderCard(true) const switchButton = screen.getByRole('switch') expect(switchButton).not.toHaveAttribute('aria-disabled', 'true') }) }) + // API keys entry point + describe('API Keys Button', () => { + it('should render the API key action', () => { + renderCard(true) + expect(screen.getByText(/serviceApi\.card\.apiKey/)).toBeInTheDocument() + }) + + it('should call onOpenSecretKeyModal when the API key action is clicked', () => { + renderCard(true) + fireEvent.click(screen.getByText(/serviceApi\.card\.apiKey/)) + expect(onOpenSecretKeyModal).toHaveBeenCalledTimes(1) + }) + }) + // Edge Cases: tests boundary scenarios describe('Edge Cases', () => { it('should handle undefined dataset id', async () => { mockDatasetId = undefined mockEnableApi.mockResolvedValue({ result: 'success' }) - render() + renderCard(false) const switchButton = screen.getByRole('switch') fireEvent.click(switchButton) diff --git a/web/app/components/datasets/extra-info/api-access/__tests__/index.spec.tsx b/web/app/components/datasets/extra-info/api-access/__tests__/index.spec.tsx index 4c504ec4771..02a588cefdb 100644 --- a/web/app/components/datasets/extra-info/api-access/__tests__/index.spec.tsx +++ b/web/app/components/datasets/extra-info/api-access/__tests__/index.spec.tsx @@ -4,11 +4,13 @@ import ApiAccess from '../index' // Mock context and hooks for Card component vi.mock('@/context/dataset-detail', () => ({ - useDatasetDetailContextWithSelector: vi.fn(() => 'test-dataset-id'), + useDatasetDetailContextWithSelector: (selector: (state: Record) => unknown) => + selector({ dataset: { id: 'test-dataset-id', permission_keys: [] }, mutateDatasetRes: () => {} }), })) vi.mock('@/context/app-context', () => ({ - useSelector: vi.fn(() => true), + useSelector: (selector: (state: Record) => unknown) => + selector({ workspacePermissionKeys: ['dataset.api_key.manage'], userProfile: { id: 'u1' } }), })) vi.mock('@/hooks/use-api-access-url', () => ({ @@ -20,6 +22,13 @@ vi.mock('@/service/knowledge/use-dataset', () => ({ useDisableDatasetServiceApi: vi.fn(() => ({ mutateAsync: vi.fn() })), })) +// Mock SecretKeyModal to avoid complex modal rendering +vi.mock('@/app/components/develop/secret-key/secret-key-modal', () => ({ + default: ({ isShow, datasetId }: { isShow: boolean, datasetId?: string }) => ( +
+ ), +})) + afterEach(() => { cleanup() }) @@ -54,6 +63,13 @@ describe('ApiAccess', () => { expect((ApiAccess as unknown as { $$typeof: symbol }).$$typeof).toBe(Symbol.for('react.memo')) }) + it('should pass the dataset id from context to the secret key modal', () => { + render() + const modal = screen.getByTestId('secret-key-modal') + expect(modal).toHaveAttribute('data-dataset-id', 'test-dataset-id') + expect(modal).toHaveAttribute('data-show', 'false') + }) + describe('toggle functionality', () => { it('should toggle open state when trigger is clicked', async () => { const { container } = render() diff --git a/web/app/components/datasets/extra-info/api-access/card.tsx b/web/app/components/datasets/extra-info/api-access/card.tsx index 3a5685e8fbb..67741a110fb 100644 --- a/web/app/components/datasets/extra-info/api-access/card.tsx +++ b/web/app/components/datasets/extra-info/api-access/card.tsx @@ -1,4 +1,5 @@ import { cn } from '@langgenius/dify-ui/cn' +import { PopoverClose } from '@langgenius/dify-ui/popover' import { StatusDot } from '@langgenius/dify-ui/status-dot' import { Switch } from '@langgenius/dify-ui/switch' import * as React from 'react' @@ -13,10 +14,12 @@ import { getDatasetACLCapabilities } from '@/utils/permission' type CardProps = { apiEnabled: boolean + onOpenSecretKeyModal: () => void } const Card = ({ apiEnabled, + onOpenSecretKeyModal, }: CardProps) => { const { t } = useTranslation() const datasetId = useDatasetDetailContextWithSelector(state => state.dataset?.id) @@ -83,6 +86,20 @@ const Card = ({
+ + +
+ {t('serviceApi.card.apiKey', { ns: 'dataset' })} +
+ + )} + /> { const { t } = useTranslation() const [open, setOpen] = useState(false) + const datasetId = useDatasetDetailContextWithSelector(state => state.dataset?.id) + const workspacePermissionKeys = useAppContextWithSelector(state => state.workspacePermissionKeys) + const canManageSecretKey = hasPermission(workspacePermissionKeys, 'dataset.api_key.manage') + const [isSecretKeyModalVisible, setIsSecretKeyModalVisible] = useState(false) + + const handleOpenSecretKeyModal = useCallback(() => { + setIsSecretKeyModalVisible(true) + }, []) + + const handleCloseSecretKeyModal = useCallback(() => { + setIsSecretKeyModalVisible(false) + }, []) return (
@@ -52,9 +68,16 @@ const ApiAccess = ({ > +
) } diff --git a/web/app/components/develop/secret-key/__tests__/secret-key-modal.spec.tsx b/web/app/components/develop/secret-key/__tests__/secret-key-modal.spec.tsx index 94c215103c6..9344011c602 100644 --- a/web/app/components/develop/secret-key/__tests__/secret-key-modal.spec.tsx +++ b/web/app/components/develop/secret-key/__tests__/secret-key-modal.spec.tsx @@ -70,6 +70,8 @@ vi.mock('@/service/use-apps', () => ({ const mockDatasetApiKeysData = vi.fn().mockReturnValue({ data: [] }) const mockIsDatasetApiKeysLoading = vi.fn().mockReturnValue(false) +const mockDatasetScopedApiKeysData = vi.fn().mockReturnValue({ data: [] }) +const mockIsDatasetScopedApiKeysLoading = vi.fn().mockReturnValue(false) const mockInvalidateDatasetApiKeys = vi.fn() vi.mock('@/service/knowledge/use-dataset', () => ({ @@ -77,6 +79,10 @@ vi.mock('@/service/knowledge/use-dataset', () => ({ data: mockDatasetApiKeysData(), isLoading: mockIsDatasetApiKeysLoading(), }), + useDatasetScopedApiKeys: (_datasetId: string | undefined, _options: unknown) => ({ + data: mockDatasetScopedApiKeysData(), + isLoading: mockIsDatasetScopedApiKeysLoading(), + }), useInvalidateDatasetApiKeys: () => mockInvalidateDatasetApiKeys, })) @@ -99,6 +105,8 @@ describe('SecretKeyModal', () => { mockIsAppApiKeysLoading.mockReturnValue(false) mockDatasetApiKeysData.mockReturnValue({ data: [] }) mockIsDatasetApiKeysLoading.mockReturnValue(false) + mockDatasetScopedApiKeysData.mockReturnValue({ data: [] }) + mockIsDatasetScopedApiKeysLoading.mockReturnValue(false) }) afterEach(() => { @@ -169,7 +177,7 @@ describe('SecretKeyModal', () => { it('should render API keys when available', async () => { await renderModal() - expect(screen.getByText('sk-...k-abc123def456ghi789')).toBeInTheDocument() + expect(screen.getByText('sk-abc123def456ghi789')).toBeInTheDocument() }) it('should render created time for keys', async () => { @@ -228,7 +236,7 @@ describe('SecretKeyModal', () => { it('should render dataset API keys when no appId', async () => { await renderModal() - expect(screen.getByText('dk-...k-abc123def456ghi789')).toBeInTheDocument() + expect(screen.getByText('dk-abc123def456ghi789')).toBeInTheDocument() }) }) @@ -284,6 +292,50 @@ describe('SecretKeyModal', () => { }) }) + it('should create a dataset-bound key by default when datasetId is provided', async () => { + const user = userEvent.setup({ advanceTimers: vi.advanceTimersByTime }) + await renderModal() + + const createButton = screen.getByText('appApi.apiKeyModal.createNewSecretKey') + await act(async () => { + await user.click(createButton) + }) + + await waitFor(() => { + expect(mockCreateDatasetApikey).toHaveBeenCalledWith({ + url: '/datasets/dataset-123/api-keys', + body: {}, + }) + }) + }) + + it('should create a workspace key when the workspace scope is selected', async () => { + const user = userEvent.setup({ advanceTimers: vi.advanceTimersByTime }) + await renderModal() + + const workspaceScopeRadio = screen.getByText('appApi.apiKeyModal.scopeAllDatasets') + await act(async () => { + await user.click(workspaceScopeRadio) + }) + + const createButton = screen.getByText('appApi.apiKeyModal.createNewSecretKey') + await act(async () => { + await user.click(createButton) + }) + + await waitFor(() => { + expect(mockCreateDatasetApikey).toHaveBeenCalledWith({ + url: '/datasets/api-keys', + body: {}, + }) + }) + }) + + it('should not render the scope selector without datasetId', async () => { + await renderModal() + expect(screen.queryByText('appApi.apiKeyModal.scopeThisDataset')).not.toBeInTheDocument() + }) + it('should show generate modal after creating key', async () => { const user = userEvent.setup({ advanceTimers: vi.advanceTimersByTime }) await renderModal() @@ -393,14 +445,14 @@ describe('SecretKeyModal', () => { it('should render delete button for permitted users', async () => { await renderModal() - const actionButtons = screen.getAllByRole('button') - expect(actionButtons.length).toBeGreaterThanOrEqual(3) + const actionButtons = document.body.querySelectorAll('button.action-btn') + expect(actionButtons.length).toBeGreaterThanOrEqual(1) }) it('should render API key row with actions', async () => { await renderModal() - expect(screen.getByText('sk-...k-abc123def456ghi789')).toBeInTheDocument() + expect(screen.getByText('sk-abc123def456ghi789')).toBeInTheDocument() }) it('should have action buttons in the key row', async () => { @@ -423,7 +475,7 @@ describe('SecretKeyModal', () => { await renderModal() const actionButtons = document.body.querySelectorAll('button.action-btn') - const deleteButton = actionButtons[1] + const deleteButton = actionButtons[0] expect(deleteButton).toBeInTheDocument() await act(async () => { @@ -443,7 +495,7 @@ describe('SecretKeyModal', () => { await renderModal() const actionButtons = document.body.querySelectorAll('button.action-btn') - const deleteButton = actionButtons[1] + const deleteButton = actionButtons[0] await act(async () => { await user.click(deleteButton!) vi.runAllTimers() @@ -473,7 +525,7 @@ describe('SecretKeyModal', () => { await renderModal() const actionButtons = document.body.querySelectorAll('button.action-btn') - const deleteButton = actionButtons[1] + const deleteButton = actionButtons[0] await act(async () => { await user.click(deleteButton!) vi.runAllTimers() @@ -500,7 +552,7 @@ describe('SecretKeyModal', () => { await renderModal() const actionButtons = document.body.querySelectorAll('button.action-btn') - const deleteButton = actionButtons[1] + const deleteButton = actionButtons[0] await act(async () => { await user.click(deleteButton!) vi.runAllTimers() @@ -529,7 +581,7 @@ describe('SecretKeyModal', () => { await renderModal() const actionButtons = document.body.querySelectorAll('button.action-btn') - const deleteButton = actionButtons[1] + const deleteButton = actionButtons[0] await act(async () => { await user.click(deleteButton!) vi.runAllTimers() @@ -565,7 +617,7 @@ describe('SecretKeyModal', () => { await renderModal() const actionButtons = document.body.querySelectorAll('button.action-btn') - const deleteButton = actionButtons[1] + const deleteButton = actionButtons[0] await act(async () => { await user.click(deleteButton!) vi.runAllTimers() @@ -590,12 +642,62 @@ describe('SecretKeyModal', () => { }) }) + it('should delete a dataset-bound key via the per-dataset route', async () => { + mockDatasetScopedApiKeysData.mockReturnValue({ + data: [ + { id: 'dk-2', token: 'dataset-bound-key-123456', dataset_id: 'dataset-123', created_at: 1700000000, last_used_at: null }, + ], + }) + const user = userEvent.setup({ advanceTimers: vi.advanceTimersByTime }) + await renderModal() + + const actionButtons = document.body.querySelectorAll('button.action-btn') + const deleteButton = actionButtons[0] + await act(async () => { + await user.click(deleteButton!) + vi.runAllTimers() + }) + + await waitFor(() => { + expect(screen.getByText('appApi.actionMsg.deleteConfirmTitle')).toBeInTheDocument() + }) + await flushTransitions() + + const confirmButton = screen.getByText('common.operation.confirm') + await act(async () => { + await user.click(confirmButton) + vi.runAllTimers() + }) + + await waitFor(() => { + expect(mockDelDatasetApikey).toHaveBeenCalledWith({ + url: '/datasets/dataset-123/api-keys/dk-2', + params: {}, + }) + }) + }) + + it('should show the scope column for dataset keys', async () => { + mockDatasetScopedApiKeysData.mockReturnValue({ + data: [ + { id: 'dk-2', token: 'dataset-bound-key-123456', dataset_id: 'dataset-123', created_at: 1700000000, last_used_at: null }, + { id: 'dk-3', token: 'dataset-workspace-key-123', dataset_id: null, created_at: 1700000000, last_used_at: null }, + ], + }) + await renderModal() + + expect(screen.getByText('appApi.apiKeyModal.scope')).toBeInTheDocument() + // Bound key: the column label plus the selected radio option both use scopeThisDataset. + expect(screen.getAllByText('appApi.apiKeyModal.scopeThisDataset').length).toBeGreaterThanOrEqual(2) + expect(screen.getAllByText('appApi.apiKeyModal.scopeAllDatasets').length).toBeGreaterThanOrEqual(2) + }) + it('should invalidate dataset API keys after deleting', async () => { const user = userEvent.setup({ advanceTimers: vi.advanceTimersByTime }) await renderModal() const actionButtons = document.body.querySelectorAll('button.action-btn') - const deleteButton = actionButtons[1] + const deleteButton = actionButtons[0] await act(async () => { await user.click(deleteButton!) vi.runAllTimers() @@ -618,16 +720,16 @@ describe('SecretKeyModal', () => { }) }) - describe('token truncation', () => { - it('should truncate token correctly', async () => { + describe('token display', () => { + it('should display the token exactly as provided by the API (no client-side reveal)', async () => { const apiKeys = [ - { id: 'key-1', token: 'sk-abcdefghijklmnopqrstuvwxyz1234567890', created_at: 1700000000, last_used_at: null }, + { id: 'key-1', token: 'sk-...7890', created_at: 1700000000, last_used_at: null }, ] mockAppApiKeysData.mockReturnValue({ data: apiKeys }) await renderModal() - expect(screen.getByText('sk-...qrstuvwxyz1234567890')).toBeInTheDocument() + expect(screen.getByText('sk-...7890')).toBeInTheDocument() }) }) diff --git a/web/app/components/develop/secret-key/secret-key-modal.tsx b/web/app/components/develop/secret-key/secret-key-modal.tsx index 13c16582e61..a6e842e5825 100644 --- a/web/app/components/develop/secret-key/secret-key-modal.tsx +++ b/web/app/components/develop/secret-key/secret-key-modal.tsx @@ -12,12 +12,13 @@ import { import { Button } from '@langgenius/dify-ui/button' import { cn } from '@langgenius/dify-ui/cn' import { Dialog, DialogContent, DialogTitle } from '@langgenius/dify-ui/dialog' +import { Radio } from '@langgenius/dify-ui/radio' +import { RadioGroup } from '@langgenius/dify-ui/radio-group' import { useState, } from 'react' import { useTranslation } from 'react-i18next' import ActionButton from '@/app/components/base/action-button' -import CopyFeedback from '@/app/components/base/copy-feedback' import Loading from '@/app/components/base/loading' import { useAppContext } from '@/context/app-context' import useTimestamp from '@/hooks/use-timestamp' @@ -29,14 +30,19 @@ import { createApikey as createDatasetApikey, delApikey as delDatasetApikey, } from '@/service/datasets' -import { useDatasetApiKeys, useInvalidateDatasetApiKeys } from '@/service/knowledge/use-dataset' +import { useDatasetApiKeys, useDatasetScopedApiKeys, useInvalidateDatasetApiKeys } from '@/service/knowledge/use-dataset' import { useAppApiKeys, useInvalidateAppApiKeys } from '@/service/use-apps' import SecretKeyGenerateModal from './secret-key-generate' import s from './style.module.css' +type DatasetKeyScope = 'dataset' | 'workspace' + type ISecretKeyModalProps = { isShow: boolean appId?: string + // When set (and appId is not), the modal manages keys for this knowledge base: + // it lists every key that can reach it and can create keys bound to it. + datasetId?: string canManage: boolean onClose: () => void } @@ -44,6 +50,7 @@ type ISecretKeyModalProps = { const SecretKeyModal = ({ isShow = false, appId, + datasetId, canManage, onClose, }: ISecretKeyModalProps) => { @@ -53,12 +60,15 @@ const SecretKeyModal = ({ const [showConfirmDelete, setShowConfirmDelete] = useState(false) const [isVisible, setIsVisible] = useState(false) const [newKey, setNewKey] = useState(undefined) + const [newKeyScope, setNewKeyScope] = useState('dataset') const invalidateAppApiKeys = useInvalidateAppApiKeys() const invalidateDatasetApiKeys = useInvalidateDatasetApiKeys() const { data: appApiKeys, isLoading: isAppApiKeysLoading } = useAppApiKeys(appId, { enabled: !!appId && isShow }) - const { data: datasetApiKeys, isLoading: isDatasetApiKeysLoading } = useDatasetApiKeys({ enabled: !appId && isShow }) - const apiKeysList = appId ? appApiKeys : datasetApiKeys - const isApiKeysLoading = appId ? isAppApiKeysLoading : isDatasetApiKeysLoading + const { data: datasetApiKeys, isLoading: isDatasetApiKeysLoading } = useDatasetApiKeys({ enabled: !appId && !datasetId && isShow }) + const { data: datasetScopedApiKeys, isLoading: isDatasetScopedApiKeysLoading } = useDatasetScopedApiKeys(datasetId, { enabled: !appId && isShow }) + const isDatasetScope = !appId && !!datasetId + const apiKeysList = appId ? appApiKeys : (isDatasetScope ? datasetScopedApiKeys : datasetApiKeys) + const isApiKeysLoading = appId ? isAppApiKeysLoading : (isDatasetScope ? isDatasetScopedApiKeysLoading : isDatasetApiKeysLoading) const [delKeyID, setDelKeyId] = useState('') @@ -69,10 +79,14 @@ const SecretKeyModal = ({ if (!delKeyID) return + const deletedKey = apiKeysList?.data?.find(api => api.id === delKeyID) const delApikey = appId ? delAppApikey : delDatasetApikey const params = appId ? { url: `/apps/${appId}/api-keys/${delKeyID}`, params: {} } - : { url: `/datasets/api-keys/${delKeyID}`, params: {} } + // Bound keys are managed on the per-dataset route; workspace keys on the tenant route. + : deletedKey?.dataset_id + ? { url: `/datasets/${deletedKey.dataset_id}/api-keys/${delKeyID}`, params: {} } + : { url: `/datasets/api-keys/${delKeyID}`, params: {} } await delApikey(params) if (appId) invalidateAppApiKeys(appId) @@ -86,7 +100,9 @@ const SecretKeyModal = ({ const params = appId ? { url: `/apps/${appId}/api-keys`, body: {} } - : { url: '/datasets/api-keys', body: {} } + : isDatasetScope && newKeyScope === 'dataset' + ? { url: `/datasets/${datasetId}/api-keys`, body: {} } + : { url: '/datasets/api-keys', body: {} } const createApikey = appId ? createAppApikey : createDatasetApikey const res = await createApikey(params) setIsVisible(true) @@ -97,8 +113,12 @@ const SecretKeyModal = ({ invalidateDatasetApiKeys() } - const generateToken = (token: string) => { - return `${token.slice(0, 3)}...${token.slice(-20)}` + const getScopeLabel = (keyDatasetId?: string | null) => { + if (!keyDatasetId) + return t('apiKeyModal.scopeAllDatasets', { ns: 'appApi' }) + return isDatasetScope + ? t('apiKeyModal.scopeThisDataset', { ns: 'appApi' }) + : t('apiKeyModal.scopeBoundDataset', { ns: 'appApi' }) } const handleDeleteConfirmOpenChange = (open: boolean) => { @@ -129,7 +149,9 @@ const SecretKeyModal = ({
- +

{t('apiKeyModal.apiSecretKeyTips', { ns: 'appApi' })}

{isApiKeysLoading &&
} @@ -137,19 +159,20 @@ const SecretKeyModal = ({ !!apiKeysList?.data?.length && (
-
{t('apiKeyModal.secretKey', { ns: 'appApi' })}
-
{t('apiKeyModal.created', { ns: 'appApi' })}
-
{t('apiKeyModal.lastUsed', { ns: 'appApi' })}
-
+
{t('apiKeyModal.secretKey', { ns: 'appApi' })}
+ {!appId &&
{t('apiKeyModal.scope', { ns: 'appApi' })}
} +
{t('apiKeyModal.created', { ns: 'appApi' })}
+
{t('apiKeyModal.lastUsed', { ns: 'appApi' })}
+
-
+
{apiKeysList.data.map(api => (
-
{generateToken(api.token)}
-
{formatTime(Number(api.created_at), t('dateTimeFormat', { ns: 'appLog' }) as string)}
-
{api.last_used_at ? formatTime(Number(api.last_used_at), t('dateTimeFormat', { ns: 'appLog' }) as string) : t('never', { ns: 'appApi' })}
-
- +
{api.token}
+ {!appId &&
{getScopeLabel(api.dataset_id)}
} +
{formatTime(Number(api.created_at), t('dateTimeFormat', { ns: 'appLog' }) as string)}
+
{api.last_used_at ? formatTime(Number(api.last_used_at), t('dateTimeFormat', { ns: 'appLog' }) as string) : t('never', { ns: 'appApi' })}
+
{canManage && ( { @@ -167,6 +190,22 @@ const SecretKeyModal = ({
) } + {isDatasetScope && ( + setNewKeyScope(value as DatasetKeyScope)} + > + + + + )}