From be6612f45468524707276d8ed61262a6bdbaebf6 Mon Sep 17 00:00:00 2001 From: yungle246 Date: Thu, 11 Jun 2026 11:41:47 +0900 Subject: [PATCH 1/9] feat: allow knowledge base API keys to be scoped to a single dataset Reintroduce the nullable api_tokens.dataset_id column (dropped in 2e9819ca5b28) so dataset API keys can opt into per-knowledge-base scoping: - NULL dataset_id keeps today's workspace-wide behavior, so every existing key and the existing /datasets/api-keys create route are unchanged. - validate_dataset_token rejects a bound key for any other dataset, and for endpoints that carry no dataset id (e.g. list-all), with 403. - CachedApiToken carries dataset_id with a None default so cache entries written before deploy keep deserializing. - The per-dataset console routes in apikey.py (previously dead code that 500ed on a missing ApiToken.dataset_id) now create bound keys; their list returns bound keys plus workspace keys so the dataset page shows the full access picture. - Frontend: the knowledge base API access popover gains an API keys entry; the secret key modal accepts datasetId, shows a scope column, and offers a workspace / this-knowledge-base scope choice on create. New strings are localized for all 23 locales. --- api/controllers/console/apikey.py | 41 ++++- api/controllers/service_api/wraps.py | 8 + ...f8a2c61d35_add_dataset_id_to_api_tokens.py | 40 +++++ api/models/model.py | 20 ++- api/services/api_token_service.py | 4 + .../controllers/console/test_apikey.py | 135 +++++++++++++++- .../console/datasets/test_datasets.py | 3 + .../controllers/console/test_apikey.py | 44 +++++- .../controllers/service_api/test_wraps.py | 130 ++++++++++++++++ .../unit_tests/libs/test_api_token_cache.py | 25 +++ .../extra-info/__tests__/index.spec.tsx | 145 ++++++++++++------ .../api-access/__tests__/card.spec.tsx | 55 +++++-- .../api-access/__tests__/index.spec.tsx | 14 ++ .../datasets/extra-info/api-access/card.tsx | 19 ++- .../datasets/extra-info/api-access/index.tsx | 20 ++- .../__tests__/secret-key-modal.spec.tsx | 102 ++++++++++++ .../develop/secret-key/secret-key-modal.tsx | 55 ++++++- web/i18n/ar-TN/app-api.json | 4 + web/i18n/de-DE/app-api.json | 4 + web/i18n/en-US/app-api.json | 4 + web/i18n/es-ES/app-api.json | 4 + web/i18n/fa-IR/app-api.json | 4 + web/i18n/fr-FR/app-api.json | 4 + web/i18n/hi-IN/app-api.json | 4 + web/i18n/id-ID/app-api.json | 4 + web/i18n/it-IT/app-api.json | 4 + web/i18n/ja-JP/app-api.json | 4 + web/i18n/ko-KR/app-api.json | 4 + web/i18n/nl-NL/app-api.json | 4 + web/i18n/pl-PL/app-api.json | 4 + web/i18n/pt-BR/app-api.json | 4 + web/i18n/ro-RO/app-api.json | 4 + web/i18n/ru-RU/app-api.json | 4 + web/i18n/sl-SI/app-api.json | 4 + web/i18n/th-TH/app-api.json | 4 + web/i18n/tr-TR/app-api.json | 4 + web/i18n/uk-UA/app-api.json | 4 + web/i18n/vi-VN/app-api.json | 4 + web/i18n/zh-Hans/app-api.json | 4 + web/i18n/zh-Hant/app-api.json | 4 + web/models/app.ts | 3 + web/service/knowledge/use-dataset.ts | 11 ++ 42 files changed, 886 insertions(+), 80 deletions(-) create mode 100644 api/migrations/versions/2026_06_10_0900-e4f8a2c61d35_add_dataset_id_to_api_tokens.py diff --git a/api/controllers/console/apikey.py b/api/controllers/console/apikey.py index 57470dc9770..d3d57e35104 100644 --- a/api/controllers/console/apikey.py +++ b/api/controllers/console/apikey.py @@ -1,11 +1,12 @@ 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 @@ -34,6 +35,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 @@ -218,30 +222,55 @@ 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 ApiKeyList.model_validate({"data": keys}, from_attributes=True) + @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") @with_current_tenant_id @edit_permission_required 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/service_api/wraps.py b/api/controllers/service_api/wraps.py index 013ea34a6ab..34b060be91d 100644 --- a/api/controllers/service_api/wraps.py +++ b/api/controllers/service_api/wraps.py @@ -262,6 +262,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_10_0900-e4f8a2c61d35_add_dataset_id_to_api_tokens.py b/api/migrations/versions/2026_06_10_0900-e4f8a2c61d35_add_dataset_id_to_api_tokens.py new file mode 100644 index 00000000000..d81df685ffb --- /dev/null +++ b/api/migrations/versions/2026_06_10_0900-e4f8a2c61d35_add_dataset_id_to_api_tokens.py @@ -0,0 +1,40 @@ +"""add dataset_id to api_tokens + +Revision ID: e4f8a2c61d35 +Revises: 7bad07dc267d +Create Date: 2026-06-10 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 = "7bad07dc267d" +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 96ab7b0e6e2..8eac8d5a322 100644 --- a/api/models/model.py +++ b/api/models/model.py @@ -2184,18 +2184,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/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..d6118f06275 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,112 @@ 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 + 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_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 101a640699f..491aa5b258d 100644 --- a/api/tests/unit_tests/controllers/console/datasets/test_datasets.py +++ b/api/tests/unit_tests/controllers/console/datasets/test_datasets.py @@ -1516,12 +1516,14 @@ class TestDatasetApiKeyApi: mock_key_1 = MagicMock(spec=ApiToken) mock_key_1.id = "key-1" mock_key_1.type = "dataset" + mock_key_1.dataset_id = None mock_key_1.token = "ds-abc" 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.dataset_id = None mock_key_2.token = "ds-def" mock_key_2.last_used_at = None mock_key_2.created_at = None @@ -1548,6 +1550,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..803fbfaef8d 100644 --- a/api/tests/unit_tests/controllers/console/test_apikey.py +++ b/api/tests/unit_tests/controllers/console/test_apikey.py @@ -9,9 +9,10 @@ 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 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 @@ -69,6 +70,7 @@ def test_list_api_keys_uses_injected_tenant_id() -> None: "id": "key-1", "type": "app", "token": "app-token", + "dataset_id": None, "last_used_at": None, "created_at": None, } @@ -106,6 +108,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/web/app/components/datasets/extra-info/__tests__/index.spec.tsx b/web/app/components/datasets/extra-info/__tests__/index.spec.tsx index de61894a11a..386f97c901d 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' @@ -529,9 +530,12 @@ describe('ApiAccessCard', () => { describe('Rendering', () => { it('should render without crashing', () => { render( - , + + + , ) expect(screen.getByText(/serviceApi\.enabled/i)).toBeInTheDocument() @@ -539,9 +543,12 @@ describe('ApiAccessCard', () => { it('should display enabled status when API is enabled', () => { render( - , + + + , ) expect(screen.getByText(/serviceApi\.enabled/i)).toBeInTheDocument() @@ -549,9 +556,12 @@ describe('ApiAccessCard', () => { it('should display disabled status when API is disabled', () => { render( - , + + + , ) expect(screen.getByText(/serviceApi\.disabled/i)).toBeInTheDocument() @@ -559,9 +569,12 @@ describe('ApiAccessCard', () => { it('should render API Reference link', () => { render( - , + + + , ) expect(screen.getByText(/overview\.apiInfo\.doc/i)).toBeInTheDocument() @@ -569,9 +582,12 @@ describe('ApiAccessCard', () => { it('should render switch component', () => { render( - , + + + , ) expect(screen.getByRole('switch')).toBeInTheDocument() @@ -583,9 +599,12 @@ describe('ApiAccessCard', () => { const user = userEvent.setup() render( - , + + + , ) const switchButton = screen.getByRole('switch') @@ -600,9 +619,12 @@ describe('ApiAccessCard', () => { const user = userEvent.setup() render( - , + + + , ) const switchButton = screen.getByRole('switch') @@ -617,9 +639,12 @@ describe('ApiAccessCard', () => { const user = userEvent.setup() render( - , + + + , ) const switchButton = screen.getByRole('switch') @@ -635,9 +660,12 @@ describe('ApiAccessCard', () => { const user = userEvent.setup() render( - , + + + , ) const switchButton = screen.getByRole('switch') @@ -653,9 +681,12 @@ describe('ApiAccessCard', () => { it('should have correct href for API Reference link', () => { render( - , + + + , ) const apiRefLink = screen.getByText(/overview\.apiInfo\.doc/i).closest('a') @@ -668,9 +699,12 @@ describe('ApiAccessCard', () => { mockIsCurrentWorkspaceManager = false render( - , + + + , ) const switchButton = screen.getByRole('switch') @@ -681,9 +715,12 @@ describe('ApiAccessCard', () => { mockIsCurrentWorkspaceManager = true render( - , + + + , ) const switchButton = screen.getByRole('switch') @@ -694,15 +731,21 @@ describe('ApiAccessCard', () => { describe('Memoization', () => { it('should be memoized with React.memo', () => { const { rerender } = render( - , + + + , ) rerender( - , + + + , ) expect(screen.getByText(/serviceApi\.enabled/i)).toBeInTheDocument() @@ -711,15 +754,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 3f9eadc7050..f3367d5910c 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 Card from '../card' @@ -36,6 +37,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() @@ -47,33 +58,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') @@ -83,13 +94,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') }) @@ -99,7 +110,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) @@ -111,7 +122,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) @@ -123,7 +134,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) @@ -135,7 +146,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) @@ -151,7 +162,7 @@ describe('Card (API Access)', () => { describe('Switch State', () => { it('should disable switch when user is not workspace manager', () => { mockIsCurrentWorkspaceManager = false - render() + renderCard(true) const switchButton = screen.getByRole('switch') expect(switchButton).toHaveAttribute('aria-checked', 'true') @@ -160,19 +171,33 @@ describe('Card (API Access)', () => { it('should enable switch when user is workspace manager', () => { mockIsCurrentWorkspaceManager = true - 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 ff866921f2c..9cfb78429b4 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 @@ -20,6 +20,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 +61,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 0b7a5778538..a503132b93e 100644 --- a/web/app/components/datasets/extra-info/api-access/card.tsx +++ b/web/app/components/datasets/extra-info/api-access/card.tsx @@ -1,7 +1,8 @@ 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 { RiArrowRightUpLine, RiBookOpenLine } from '@remixicon/react' +import { RiArrowRightUpLine, RiBookOpenLine, RiKey2Line } from '@remixicon/react' import * as React from 'react' import { useCallback } from 'react' import { useTranslation } from 'react-i18next' @@ -13,10 +14,12 @@ import { useDisableDatasetServiceApi, useEnableDatasetServiceApi } from '@/servi type CardProps = { apiEnabled: boolean + onOpenSecretKeyModal: () => void } const Card = ({ apiEnabled, + onOpenSecretKeyModal, }: CardProps) => { const { t } = useTranslation() const datasetId = useDatasetDetailContextWithSelector(state => state.dataset?.id) @@ -72,6 +75,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 [isSecretKeyModalVisible, setIsSecretKeyModalVisible] = useState(false) + + const handleOpenSecretKeyModal = useCallback(() => { + setIsSecretKeyModalVisible(true) + }, []) + + const handleCloseSecretKeyModal = useCallback(() => { + setIsSecretKeyModalVisible(false) + }, []) return (
@@ -52,9 +64,15 @@ 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 00948180eb4..84f3eb13fed 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, })) @@ -98,6 +104,8 @@ describe('SecretKeyModal', () => { mockIsAppApiKeysLoading.mockReturnValue(false) mockDatasetApiKeysData.mockReturnValue({ data: [] }) mockIsDatasetApiKeysLoading.mockReturnValue(false) + mockDatasetScopedApiKeysData.mockReturnValue({ data: [] }) + mockIsDatasetScopedApiKeysLoading.mockReturnValue(false) }) afterEach(() => { @@ -276,6 +284,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() @@ -574,6 +626,56 @@ 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[1] + 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() 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 7d949d7864e..46c61da006b 100644 --- a/web/app/components/develop/secret-key/secret-key-modal.tsx +++ b/web/app/components/develop/secret-key/secret-key-modal.tsx @@ -13,6 +13,8 @@ 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 { RiDeleteBinLine } from '@remixicon/react' import { useState, @@ -31,20 +33,26 @@ 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 onClose: () => void } const SecretKeyModal = ({ isShow = false, appId, + datasetId, onClose, }: ISecretKeyModalProps) => { const { t } = useTranslation() @@ -53,12 +61,15 @@ const SecretKeyModal = ({ const [showConfirmDelete, setShowConfirmDelete] = useState(false) const [isVisible, setVisible] = 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('') @@ -67,10 +78,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) @@ -81,7 +96,9 @@ const SecretKeyModal = ({ const onCreate = async () => { 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) setVisible(true) @@ -92,6 +109,14 @@ const SecretKeyModal = ({ invalidateDatasetApiKeys() } + 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 generateToken = (token: string) => { return `${token.slice(0, 3)}...${token.slice(-20)}` } @@ -133,6 +158,7 @@ const SecretKeyModal = ({
{t('apiKeyModal.secretKey', { ns: 'appApi' })}
+ {!appId &&
{t('apiKeyModal.scope', { ns: 'appApi' })}
}
{t('apiKeyModal.created', { ns: 'appApi' })}
{t('apiKeyModal.lastUsed', { ns: 'appApi' })}
@@ -141,6 +167,7 @@ const SecretKeyModal = ({ {apiKeysList.data.map(api => (
{generateToken(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' })}
@@ -162,6 +189,22 @@ const SecretKeyModal = ({
) } + {isDatasetScope && ( + setNewKeyScope(value as DatasetKeyScope)} + > + + + + )}

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

{isApiKeysLoading &&
} @@ -171,12 +168,11 @@ const SecretKeyModal = ({
{apiKeysList.data.map(api => (
-
{generateToken(api.token)}
+
{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 && ( { From d3c62d96481df27d22a07f86b9cc472a895a092c Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Thu, 25 Jun 2026 05:26:24 +0000 Subject: [PATCH 9/9] [autofix.ci] apply automated fixes --- eslint-suppressions.json | 8 -------- 1 file changed, 8 deletions(-) diff --git a/eslint-suppressions.json b/eslint-suppressions.json index a9975b4476e..5c849574f34 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -3492,14 +3492,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