From 86497045c98b4dd27501d3f510f358d7aecabf41 Mon Sep 17 00:00:00 2001 From: YungLe <143590624+ForeignKeyCN@users.noreply.github.com> Date: Mon, 1 Jun 2026 14:56:18 +0900 Subject: [PATCH] feat: per-credential visibility control for plugin credentials (#35468) Co-authored-by: Yang Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) --- .gitignore | 3 + .../datasets/rag_pipeline/datasource_auth.py | 3 +- api/controllers/console/workspace/models.py | 7 +- .../console/workspace/tool_providers.py | 21 +- .../console/workspace/trigger_providers.py | 5 +- api/core/provider_manager.py | 23 +- api/core/tools/entities/api_entities.py | 18 ++ ...-a1b2c3d4e5f6_add_credential_visibility.py | 121 ++++++++ api/models/__init__.py | 6 + api/models/credential_permission.py | 53 ++++ api/models/dataset.py | 9 +- api/models/enums.py | 8 + api/models/oauth.py | 10 +- api/models/provider.py | 9 +- api/models/tools.py | 7 + api/models/trigger.py | 8 +- api/openapi/markdown/console-swagger.md | 19 +- api/openapi/markdown/service-swagger.md | 18 +- api/services/credential_permission_service.py | 71 +++++ api/services/datasource_provider_service.py | 45 ++- api/services/model_provider_service.py | 8 +- .../tools/builtin_tools_manage_service.py | 113 ++++++- .../trigger/trigger_provider_service.py | 31 +- .../console/workspace/test_tool_provider.py | 4 +- .../console/workspace/test_tool_providers.py | 16 +- .../test_credential_permission_service.py | 103 +++++++ eslint-suppressions.json | 13 +- .../api/console/datasets/types.gen.ts | 6 +- .../generated/api/console/datasets/zod.gen.ts | 10 +- .../api/console/workspaces/types.gen.ts | 1 + .../api/console/workspaces/zod.gen.ts | 1 + .../generated/api/service/types.gen.ts | 8 +- .../generated/api/service/zod.gen.ts | 16 +- .../base/permission-selector/index.tsx | 279 ++++++++++++++++++ .../base/permission-selector/member-item.tsx | 49 +++ .../permission-selector/permission-item.tsx | 31 ++ .../__tests__/authorized-in-node.spec.tsx | 4 + .../__tests__/plugin-auth-in-agent.spec.tsx | 5 + .../__tests__/api-key-modal.spec.tsx | 12 + .../authorize/add-api-key-button.tsx | 20 +- .../plugin-auth/authorize/api-key-modal.tsx | 41 ++- .../plugins/plugin-auth/authorize/index.tsx | 12 +- .../plugin-auth/authorized-in-node.tsx | 2 +- .../authorized/__tests__/item.spec.tsx | 19 +- .../plugins/plugin-auth/authorized/index.tsx | 20 ++ .../plugins/plugin-auth/authorized/item.tsx | 44 ++- .../plugin-auth/hooks/use-credential.ts | 16 +- .../plugin-auth/hooks/use-plugin-auth.ts | 8 +- .../plugin-auth/plugin-auth-in-agent.tsx | 2 +- .../components/plugins/plugin-auth/types.ts | 12 +- web/i18n/en-US/plugin.json | 6 + web/i18n/zh-Hans/plugin.json | 6 + web/models/datasets.ts | 10 +- web/models/permission.ts | 11 + web/service/use-plugins-auth.ts | 4 + 55 files changed, 1282 insertions(+), 125 deletions(-) create mode 100644 api/migrations/versions/2026_04_11_1400-a1b2c3d4e5f6_add_credential_visibility.py create mode 100644 api/models/credential_permission.py create mode 100644 api/services/credential_permission_service.py create mode 100644 api/tests/unit_tests/services/test_credential_permission_service.py create mode 100644 web/app/components/base/permission-selector/index.tsx create mode 100644 web/app/components/base/permission-selector/member-item.tsx create mode 100644 web/app/components/base/permission-selector/permission-item.tsx create mode 100644 web/models/permission.ts diff --git a/.gitignore b/.gitignore index 5b434ee4ec..8a4418e847 100644 --- a/.gitignore +++ b/.gitignore @@ -259,3 +259,6 @@ scripts/stress-test/reports/ .qoder/* .context/ .eslintcache + +# Vitest local reports +web/.vitest-reports/ diff --git a/api/controllers/console/datasets/rag_pipeline/datasource_auth.py b/api/controllers/console/datasets/rag_pipeline/datasource_auth.py index f6dcd0e492..6445f06297 100644 --- a/api/controllers/console/datasets/rag_pipeline/datasource_auth.py +++ b/api/controllers/console/datasets/rag_pipeline/datasource_auth.py @@ -198,12 +198,13 @@ class DatasourceAuth(Resource): def get(self, provider_id: str): datasource_provider_id = DatasourceProviderID(provider_id) datasource_provider_service = DatasourceProviderService() - _, current_tenant_id = current_account_with_tenant() + user, current_tenant_id = current_account_with_tenant() datasources = datasource_provider_service.list_datasource_credentials( tenant_id=current_tenant_id, provider=datasource_provider_id.provider_name, plugin_id=datasource_provider_id.plugin_id, + user=user, ) return {"result": datasources}, 200 diff --git a/api/controllers/console/workspace/models.py b/api/controllers/console/workspace/models.py index 6c703d782d..d0efb9f369 100644 --- a/api/controllers/console/workspace/models.py +++ b/api/controllers/console/workspace/models.py @@ -18,7 +18,7 @@ from graphon.model_runtime.entities.model_entities import ModelType from graphon.model_runtime.errors.validate import CredentialsValidateFailedError from graphon.model_runtime.utils.encoders import jsonable_encoder from libs.helper import uuid_value -from libs.login import login_required +from libs.login import current_account_with_tenant, login_required from services.model_load_balancing_service import ModelLoadBalancingService from services.model_provider_service import ModelProviderService @@ -292,9 +292,14 @@ class ModelProviderModelCredentialApi(Resource): ) if args.config_from == "predefined-model": + # Only the predefined-model branch needs visibility filtering by user. + # Defer the auth lookup so the other branch (and its tests) doesn't + # require flask-login setup. + user, _ = current_account_with_tenant() available_credentials = model_provider_service.get_provider_available_credentials( tenant_id=tenant_id, provider=provider, + user=user, ) else: available_credentials = model_provider_service.get_provider_model_available_credentials( diff --git a/api/controllers/console/workspace/tool_providers.py b/api/controllers/console/workspace/tool_providers.py index 4ef4b02118..d05d91ffca 100644 --- a/api/controllers/console/workspace/tool_providers.py +++ b/api/controllers/console/workspace/tool_providers.py @@ -69,6 +69,7 @@ class BuiltinToolAddPayload(BaseModel): credentials: dict[str, Any] name: str | None = Field(default=None, max_length=30) type: CredentialType + visibility: str | None = None class BuiltinToolUpdatePayload(BaseModel): @@ -338,6 +339,7 @@ class ToolBuiltinProviderAddApi(Resource): credentials=payload.credentials, name=payload.name, api_type=CredentialType.of(payload.type), + visibility=payload.visibility, ) @@ -371,12 +373,19 @@ class ToolBuiltinProviderGetCredentialsApi(Resource): @login_required @account_initialization_required def get(self, provider): - _, tenant_id = current_account_with_tenant() + user, tenant_id = current_account_with_tenant() + # Optional list of credential IDs to include even if visibility would hide them + # (used when a workflow/agent node still references another member's only_me credential). + include_credential_ids = request.args.getlist("include_credential_ids") or [ + s for s in (request.args.get("include_credential_ids") or "").split(",") if s + ] return jsonable_encoder( BuiltinToolManageService.get_builtin_tool_provider_credentials( tenant_id=tenant_id, provider_name=provider, + user=user, + include_credential_ids=include_credential_ids or None, ) ) @@ -859,7 +868,7 @@ class ToolOAuthCallback(Resource): if not credentials: raise Exception("the plugin credentials failed") - # add credentials to database + # add credentials to database — OAuth tokens default to only_me since they're personal BuiltinToolManageService.add_builtin_tool_provider( user_id=user_id, tenant_id=tenant_id, @@ -867,6 +876,7 @@ class ToolOAuthCallback(Resource): credentials=dict(credentials), expires_at=expires_at, api_type=CredentialType.OAUTH2, + visibility="only_me", ) return redirect(f"{dify_config.CONSOLE_WEB_URL}/oauth-callback") @@ -946,12 +956,17 @@ class ToolBuiltinProviderGetCredentialInfoApi(Resource): @login_required @account_initialization_required def get(self, provider): - _, tenant_id = current_account_with_tenant() + user, tenant_id = current_account_with_tenant() + include_credential_ids = request.args.getlist("include_credential_ids") or [ + s for s in (request.args.get("include_credential_ids") or "").split(",") if s + ] return jsonable_encoder( BuiltinToolManageService.get_builtin_tool_provider_credential_info( tenant_id=tenant_id, provider=provider, + user=user, + include_credential_ids=include_credential_ids or None, ) ) diff --git a/api/controllers/console/workspace/trigger_providers.py b/api/controllers/console/workspace/trigger_providers.py index ab44e46ed2..b3c790f7d7 100644 --- a/api/controllers/console/workspace/trigger_providers.py +++ b/api/controllers/console/workspace/trigger_providers.py @@ -122,12 +122,15 @@ class TriggerSubscriptionListApi(Resource): def get(self, provider): """List all trigger subscriptions for the current tenant's provider""" user = current_user + assert isinstance(user, Account) assert user.current_tenant_id is not None try: return jsonable_encoder( TriggerProviderService.list_trigger_provider_subscriptions( - tenant_id=user.current_tenant_id, provider_id=TriggerProviderID(provider) + tenant_id=user.current_tenant_id, + provider_id=TriggerProviderID(provider), + user=user, ) ) except ValueError as e: diff --git a/api/core/provider_manager.py b/api/core/provider_manager.py index 0ba668a5e8..5cb61d0c7b 100644 --- a/api/core/provider_manager.py +++ b/api/core/provider_manager.py @@ -57,6 +57,7 @@ from services.feature_service import FeatureService if TYPE_CHECKING: from graphon.model_runtime.protocols.runtime import ModelRuntime + from models.account import Account _credentials_adapter: TypeAdapter[dict[str, Any]] = TypeAdapter(dict[str, Any]) @@ -572,14 +573,22 @@ class ProviderManager: return provider_names @staticmethod - def get_provider_available_credentials(tenant_id: str, provider_name: str) -> list[CredentialConfiguration]: + def get_provider_available_credentials( + tenant_id: str, + provider_name: str, + user: Account | None = None, + ) -> list[CredentialConfiguration]: """ - Get provider all credentials. + Get provider all credentials, filtered by visibility. :param tenant_id: workspace id :param provider_name: provider name + :param user: current user (id + admin flag drive the visibility filter) :return: """ + from models.credential_permission import CredentialType as CredPermType + from services.credential_permission_service import CredentialPermissionService + with session_factory.create_session() as session: stmt = ( select(ProviderCredential) @@ -590,6 +599,16 @@ class ProviderManager: .order_by(ProviderCredential.created_at.desc()) ) + if user is not None: + stmt = CredentialPermissionService.apply_visibility_filter( + stmt, + model_id_column=ProviderCredential.id, + model_user_id_column=ProviderCredential.user_id, + model_visibility_column=ProviderCredential.visibility, + credential_type=CredPermType.PROVIDER_CREDENTIAL, + user=user, + ) + available_credentials = session.scalars(stmt).all() return [ diff --git a/api/core/tools/entities/api_entities.py b/api/core/tools/entities/api_entities.py index 42a88c0003..a80fe9b663 100644 --- a/api/core/tools/entities/api_entities.py +++ b/api/core/tools/entities/api_entities.py @@ -129,6 +129,24 @@ class ToolProviderCredentialApiEntity(BaseModel): default=False, description="Whether the credential is the default credential for the provider in the workspace" ) credentials: Mapping[str, object] = Field(description="The credentials of the provider", default_factory=dict) + visibility: str = Field( + default="all_team_members", + description="Credential visibility: only_me, all_team_members, or partial_members", + ) + created_by: str = Field(default="", description="User ID of the credential creator") + partial_member_list: list[str] = Field( + default_factory=list, + description="List of user IDs allowed when visibility is partial_members", + ) + from_other_member: bool = Field( + default=False, + description=( + "True when this credential is being returned only because a workflow/agent node still " + "references it but it would normally be hidden from this user by the visibility filter " + "(another member's only_me credential). The frontend renders it as 'borrowed' — " + "selectable until the node switches away, but not editable/deletable." + ), + ) class ToolProviderCredentialInfoApiEntity(BaseModel): diff --git a/api/migrations/versions/2026_04_11_1400-a1b2c3d4e5f6_add_credential_visibility.py b/api/migrations/versions/2026_04_11_1400-a1b2c3d4e5f6_add_credential_visibility.py new file mode 100644 index 0000000000..f079791104 --- /dev/null +++ b/api/migrations/versions/2026_04_11_1400-a1b2c3d4e5f6_add_credential_visibility.py @@ -0,0 +1,121 @@ +"""add credential visibility and permission table + +Revision ID: a1b2c3d4e5f6 +Revises: 7885bd53f9a9 +Create Date: 2026-04-11 14:00:00.000000 + +""" + +import sqlalchemy as sa +from alembic import op + +import models as models + +# revision identifiers, used by Alembic. +revision = "a1b2c3d4e5f6" +down_revision = "7885bd53f9a9" +branch_labels = None +depends_on = None + + +def _is_pg(conn): + return conn.dialect.name == "postgresql" + + +def upgrade(): + conn = op.get_bind() + + # 1. Add visibility column to trigger_subscriptions + with op.batch_alter_table("trigger_subscriptions", schema=None) as batch_op: + batch_op.add_column( + sa.Column("visibility", sa.String(length=40), nullable=False, server_default="all_team_members") + ) + + # 2. Add visibility column to tool_builtin_providers + with op.batch_alter_table("tool_builtin_providers", schema=None) as batch_op: + batch_op.add_column( + sa.Column("visibility", sa.String(length=40), nullable=False, server_default="all_team_members") + ) + + # 3. Add user_id + visibility to datasource_providers + with op.batch_alter_table("datasource_providers", schema=None) as batch_op: + batch_op.add_column(sa.Column("user_id", models.types.StringUUID(), nullable=True)) + batch_op.add_column( + sa.Column("visibility", sa.String(length=40), nullable=False, server_default="all_team_members") + ) + + # 4. Add user_id + visibility to provider_credentials + with op.batch_alter_table("provider_credentials", schema=None) as batch_op: + batch_op.add_column(sa.Column("user_id", models.types.StringUUID(), nullable=True)) + batch_op.add_column( + sa.Column("visibility", sa.String(length=40), nullable=False, server_default="all_team_members") + ) + + # 5. Create credential_permissions table + if _is_pg(conn): + op.create_table( + "credential_permissions", + sa.Column( + "id", models.types.StringUUID(), server_default=sa.text("uuid_generate_v4()"), nullable=False + ), + sa.Column("credential_id", models.types.StringUUID(), nullable=False), + sa.Column("credential_type", sa.String(length=40), nullable=False), + sa.Column("account_id", models.types.StringUUID(), nullable=False), + sa.Column("tenant_id", models.types.StringUUID(), nullable=False), + sa.Column("has_permission", sa.Boolean(), nullable=False, server_default=sa.text("true")), + sa.Column( + "created_at", sa.DateTime(), server_default=sa.text("CURRENT_TIMESTAMP"), nullable=False + ), + sa.PrimaryKeyConstraint("id", name="credential_permission_pkey"), + ) + else: + op.create_table( + "credential_permissions", + sa.Column("id", models.types.StringUUID(), nullable=False), + sa.Column("credential_id", models.types.StringUUID(), nullable=False), + sa.Column("credential_type", sa.String(length=40), nullable=False), + sa.Column("account_id", models.types.StringUUID(), nullable=False), + sa.Column("tenant_id", models.types.StringUUID(), nullable=False), + sa.Column("has_permission", sa.Boolean(), nullable=False, server_default=sa.text("1")), + sa.Column( + "created_at", + sa.DateTime(), + server_default=sa.func.current_timestamp(), + nullable=False, + ), + sa.PrimaryKeyConstraint("id", name="credential_permission_pkey"), + ) + + with op.batch_alter_table("credential_permissions", schema=None) as batch_op: + batch_op.create_index( + "idx_credential_permissions_credential", ["credential_id", "credential_type"], unique=False + ) + batch_op.create_index("idx_credential_permissions_account_id", ["account_id"], unique=False) + batch_op.create_index("idx_credential_permissions_tenant_id", ["tenant_id"], unique=False) + + +def downgrade(): + # Drop credential_permissions table + with op.batch_alter_table("credential_permissions", schema=None) as batch_op: + batch_op.drop_index("idx_credential_permissions_tenant_id") + batch_op.drop_index("idx_credential_permissions_account_id") + batch_op.drop_index("idx_credential_permissions_credential") + op.drop_table("credential_permissions") + + # Remove visibility from trigger_subscriptions + with op.batch_alter_table("trigger_subscriptions", schema=None) as batch_op: + batch_op.drop_column("visibility") + + # Remove visibility from tool_builtin_providers + with op.batch_alter_table("tool_builtin_providers", schema=None) as batch_op: + batch_op.drop_column("visibility") + + # Remove user_id + visibility from datasource_providers + with op.batch_alter_table("datasource_providers", schema=None) as batch_op: + batch_op.drop_column("visibility") + batch_op.drop_column("user_id") + + # Remove user_id + visibility from provider_credentials + with op.batch_alter_table("provider_credentials", schema=None) as batch_op: + batch_op.drop_column("visibility") + batch_op.drop_column("user_id") diff --git a/api/models/__init__.py b/api/models/__init__.py index 47962cad21..c87acab04e 100644 --- a/api/models/__init__.py +++ b/api/models/__init__.py @@ -29,6 +29,8 @@ from .comment import ( WorkflowCommentMention, WorkflowCommentReply, ) +from .credential_permission import CredentialPermission +from .credential_permission import CredentialType as CredentialPermissionType from .dataset import ( AppDatasetJoin, Dataset, @@ -50,6 +52,7 @@ from .enums import ( AppTriggerStatus, AppTriggerType, CreatorUserRole, + PermissionEnum, WorkflowRunTriggeredFrom, WorkflowTriggerStatus, ) @@ -168,6 +171,8 @@ __all__ = [ "Conversation", "ConversationVariable", "CreatorUserRole", + "CredentialPermission", + "CredentialPermissionType", "DataSourceApiKeyAuthBinding", "DataSourceOauthBinding", "Dataset", @@ -203,6 +208,7 @@ __all__ = [ "MessageFile", "OAuthAccessToken", "OperationLog", + "PermissionEnum", "PinnedConversation", "Provider", "ProviderModel", diff --git a/api/models/credential_permission.py b/api/models/credential_permission.py new file mode 100644 index 0000000000..effa78635b --- /dev/null +++ b/api/models/credential_permission.py @@ -0,0 +1,53 @@ +from datetime import datetime +from enum import StrEnum +from uuid import uuid4 + +import sqlalchemy as sa +from sqlalchemy import DateTime, String, func +from sqlalchemy.orm import Mapped, mapped_column + +from .base import TypeBase +from .types import StringUUID + + +class CredentialType(StrEnum): + """Discriminator for polymorphic credential permission table.""" + + TRIGGER_SUBSCRIPTION = "trigger_subscription" + BUILTIN_TOOL_PROVIDER = "builtin_tool_provider" + DATASOURCE_PROVIDER = "datasource_provider" + PROVIDER_CREDENTIAL = "provider_credential" + + +class CredentialPermission(TypeBase): + """ + Polymorphic join table for per-credential partial-member access control. + Mirrors DatasetPermission (api/models/dataset.py) but supports all credential types + via a credential_type discriminator column. + """ + + __tablename__ = "credential_permissions" + __table_args__ = ( + sa.PrimaryKeyConstraint("id", name="credential_permission_pkey"), + sa.Index("idx_credential_permissions_credential", "credential_id", "credential_type"), + sa.Index("idx_credential_permissions_account_id", "account_id"), + sa.Index("idx_credential_permissions_tenant_id", "tenant_id"), + ) + + id: Mapped[str] = mapped_column( + StringUUID, + insert_default=lambda: str(uuid4()), + default_factory=lambda: str(uuid4()), + primary_key=True, + init=False, + ) + credential_id: Mapped[str] = mapped_column(StringUUID, nullable=False) + credential_type: Mapped[str] = mapped_column(String(40), nullable=False) + account_id: Mapped[str] = mapped_column(StringUUID, nullable=False) + tenant_id: Mapped[str] = mapped_column(StringUUID, nullable=False) + has_permission: Mapped[bool] = mapped_column( + sa.Boolean, nullable=False, server_default=sa.text("true"), default=True + ) + created_at: Mapped[datetime] = mapped_column( + DateTime, nullable=False, server_default=func.current_timestamp(), init=False + ) diff --git a/api/models/dataset.py b/api/models/dataset.py index 8137ed4ff3..0f26752dd3 100644 --- a/api/models/dataset.py +++ b/api/models/dataset.py @@ -1,5 +1,4 @@ import base64 -import enum import hashlib import hmac import json @@ -157,10 +156,10 @@ class DocumentDict(TypedDict): hit_count: int | None -class DatasetPermissionEnum(enum.StrEnum): - ONLY_ME = "only_me" - ALL_TEAM = "all_team_members" - PARTIAL_TEAM = "partial_members" +from models.enums import PermissionEnum + +# Backward-compatible alias — new code should import PermissionEnum from models.enums +DatasetPermissionEnum = PermissionEnum class Dataset(Base): diff --git a/api/models/enums.py b/api/models/enums.py index 34d7c35ac5..3b22d5a550 100644 --- a/api/models/enums.py +++ b/api/models/enums.py @@ -356,3 +356,11 @@ class ApiTokenType(StrEnum): APP = "app" DATASET = "dataset" + + +class PermissionEnum(StrEnum): + """Shared permission levels for resources (datasets, credentials, etc.)""" + + ONLY_ME = "only_me" + ALL_TEAM = "all_team_members" + PARTIAL_TEAM = "partial_members" diff --git a/api/models/oauth.py b/api/models/oauth.py index f17a4d3342..84be7eb37c 100644 --- a/api/models/oauth.py +++ b/api/models/oauth.py @@ -8,7 +8,8 @@ from sqlalchemy.orm import Mapped, mapped_column from libs.uuid_utils import uuidv7 from .base import TypeBase -from .types import AdjustedJSON, LongText, StringUUID +from .enums import PermissionEnum +from .types import AdjustedJSON, EnumText, LongText, StringUUID class DatasourceOauthParamConfig(TypeBase): @@ -42,9 +43,16 @@ class DatasourceProvider(TypeBase): plugin_id: Mapped[str] = mapped_column(sa.String(255), nullable=False) auth_type: Mapped[str] = mapped_column(sa.String(255), nullable=False) encrypted_credentials: Mapped[dict[str, Any]] = mapped_column(AdjustedJSON, nullable=False) + user_id: Mapped[str | None] = mapped_column(StringUUID, nullable=True, default=None) avatar_url: Mapped[str] = mapped_column(LongText, nullable=True, default="default") is_default: Mapped[bool] = mapped_column(sa.Boolean, nullable=False, server_default=sa.text("false"), default=False) expires_at: Mapped[int] = mapped_column(sa.Integer, nullable=False, server_default="-1", default=-1) + visibility: Mapped[PermissionEnum] = mapped_column( + EnumText(PermissionEnum, length=40), + nullable=False, + server_default=sa.text("'all_team_members'"), + default=PermissionEnum.ALL_TEAM, + ) created_at: Mapped[datetime] = mapped_column( sa.DateTime, nullable=False, server_default=func.current_timestamp(), init=False diff --git a/api/models/provider.py b/api/models/provider.py index 8dc3ce4ff6..0bc2fc2130 100644 --- a/api/models/provider.py +++ b/api/models/provider.py @@ -14,7 +14,7 @@ from graphon.model_runtime.entities.model_entities import ModelType from libs.uuid_utils import uuidv7 from .base import TypeBase -from .enums import CredentialSourceType, PaymentStatus, ProviderQuotaType +from .enums import CredentialSourceType, PaymentStatus, PermissionEnum, ProviderQuotaType from .types import EnumText, LongText, StringUUID @@ -320,6 +320,13 @@ class ProviderCredential(TypeBase): provider_name: Mapped[str] = mapped_column(String(255), nullable=False) credential_name: Mapped[str] = mapped_column(String(255), nullable=False) encrypted_config: Mapped[str] = mapped_column(LongText, nullable=False) + user_id: Mapped[str | None] = mapped_column(StringUUID, nullable=True, default=None) + visibility: Mapped[PermissionEnum] = mapped_column( + EnumText(PermissionEnum, length=40), + nullable=False, + server_default=sa.text("'all_team_members'"), + default=PermissionEnum.ALL_TEAM, + ) created_at: Mapped[datetime] = mapped_column( DateTime, nullable=False, server_default=func.current_timestamp(), init=False ) diff --git a/api/models/tools.py b/api/models/tools.py index 02f8b5217d..b9e66d2b77 100644 --- a/api/models/tools.py +++ b/api/models/tools.py @@ -22,6 +22,7 @@ from core.tools.entities.tool_entities import ( from .base import TypeBase from .engine import db +from .enums import PermissionEnum from .model import Account, App, Tenant from .types import EnumText, LongText, StringUUID @@ -117,6 +118,12 @@ class BuiltinToolProvider(TypeBase): default=CredentialType.API_KEY, ) expires_at: Mapped[int] = mapped_column(sa.BigInteger, nullable=False, server_default=sa.text("-1"), default=-1) + visibility: Mapped[PermissionEnum] = mapped_column( + EnumText(PermissionEnum, length=40), + nullable=False, + server_default=sa.text("'all_team_members'"), + default=PermissionEnum.ALL_TEAM, + ) @property def credentials(self) -> dict[str, Any]: diff --git a/api/models/trigger.py b/api/models/trigger.py index 5233a6e271..69df086638 100644 --- a/api/models/trigger.py +++ b/api/models/trigger.py @@ -19,7 +19,7 @@ from libs.uuid_utils import uuidv7 from .base import TypeBase from .engine import db -from .enums import AppTriggerStatus, AppTriggerType, CreatorUserRole, WorkflowTriggerStatus +from .enums import AppTriggerStatus, AppTriggerType, CreatorUserRole, PermissionEnum, WorkflowTriggerStatus from .model import Account from .types import EnumText, LongText, StringUUID @@ -111,6 +111,12 @@ class TriggerSubscription(TypeBase): expires_at: Mapped[int] = mapped_column( Integer, default=-1, comment="Subscription instance expiration timestamp, -1 for never" ) + visibility: Mapped[PermissionEnum] = mapped_column( + EnumText(PermissionEnum, length=40), + nullable=False, + server_default=sa.text("'all_team_members'"), + default=PermissionEnum.ALL_TEAM, + ) created_at: Mapped[datetime] = mapped_column( DateTime, nullable=False, server_default=func.current_timestamp(), init=False diff --git a/api/openapi/markdown/console-swagger.md b/api/openapi/markdown/console-swagger.md index b76172e996..caaa0216e2 100644 --- a/api/openapi/markdown/console-swagger.md +++ b/api/openapi/markdown/console-swagger.md @@ -11662,6 +11662,7 @@ Retrieval settings for Amazon Bedrock knowledge base queries. | credentials | object | | Yes | | name | string | | No | | type | [CredentialType](#credentialtype) | | Yes | +| visibility | string | | No | #### BuiltinToolCredentialDeletePayload @@ -12233,7 +12234,7 @@ Condition detail | external_knowledge_id | string | | No | | indexing_technique | string | | No | | name | string | | Yes | -| permission | [DatasetPermissionEnum](#datasetpermissionenum) | | No | +| permission | [PermissionEnum](#permissionenum) | | No | | provider | string | | No | #### DatasetDetail @@ -12508,12 +12509,6 @@ Condition detail | name | string | | Yes | | type | string | | Yes | -#### DatasetPermissionEnum - -| Name | Type | Description | Required | -| ---- | ---- | ----------- | -------- | -| DatasetPermissionEnum | string | | | - #### DatasetQueryContentResponse | Name | Type | Description | Required | @@ -12640,7 +12635,7 @@ Condition detail | is_multimodal | boolean | | No | | name | string | | No | | partial_member_list | [ object ] | | No | -| permission | [DatasetPermissionEnum](#datasetpermissionenum) | | No | +| permission | [PermissionEnum](#permissionenum) | | No | | retrieval_model | object | | No | | summary_index_setting | object | | No | @@ -14609,6 +14604,14 @@ Form input definition. | icon_info | object | | No | | name | string | | Yes | +#### PermissionEnum + +Shared permission levels for resources (datasets, credentials, etc.) + +| Name | Type | Description | Required | +| ---- | ---- | ----------- | -------- | +| PermissionEnum | string | Shared permission levels for resources (datasets, credentials, etc.) | | + #### PipelineVariableResponse | Name | Type | Description | Required | diff --git a/api/openapi/markdown/service-swagger.md b/api/openapi/markdown/service-swagger.md index 5105112b5a..5bdb526445 100644 --- a/api/openapi/markdown/service-swagger.md +++ b/api/openapi/markdown/service-swagger.md @@ -2378,7 +2378,7 @@ Condition detail | external_knowledge_id | string | | No | | indexing_technique | string | *Enum:* `"economy"`, `"high_quality"` | No | | name | string | | Yes | -| permission | [DatasetPermissionEnum](#datasetpermissionenum) | | No | +| permission | [PermissionEnum](#permissionenum) | | No | | provider | string | | No | | retrieval_model | [RetrievalModel](#retrievalmodel) | | No | | summary_index_setting | object | | No | @@ -2567,12 +2567,6 @@ Condition detail | name | string | | Yes | | type | string | | Yes | -#### DatasetPermissionEnum - -| Name | Type | Description | Required | -| ---- | ---- | ----------- | -------- | -| DatasetPermissionEnum | string | | | - #### DatasetRerankingModelResponse | Name | Type | Description | Required | @@ -2623,7 +2617,7 @@ Condition detail | indexing_technique | string | *Enum:* `"economy"`, `"high_quality"` | No | | name | string | | No | | partial_member_list | [ object ] | | No | -| permission | [DatasetPermissionEnum](#datasetpermissionenum) | | No | +| permission | [PermissionEnum](#permissionenum) | | No | | retrieval_model | [RetrievalModel](#retrievalmodel) | | No | #### DatasetVectorSettingResponse @@ -3013,6 +3007,14 @@ Metadata operation data | ---- | ---- | ----------- | -------- | | name | string | | Yes | +#### PermissionEnum + +Shared permission levels for resources (datasets, credentials, etc.) + +| Name | Type | Description | Required | +| ---- | ---- | ----------- | -------- | +| PermissionEnum | string | Shared permission levels for resources (datasets, credentials, etc.) | | + #### PipelineRunApiEntity | Name | Type | Description | Required | diff --git a/api/services/credential_permission_service.py b/api/services/credential_permission_service.py new file mode 100644 index 0000000000..21806e0178 --- /dev/null +++ b/api/services/credential_permission_service.py @@ -0,0 +1,71 @@ +from collections.abc import Sequence + +from sqlalchemy import or_, select +from sqlalchemy.orm import InstrumentedAttribute + +from extensions.ext_database import db +from models.account import Account +from models.credential_permission import CredentialPermission +from models.enums import PermissionEnum + + +class CredentialPermissionService: + """ + Shared service for per-credential access control. + Mirrors DatasetPermissionService but supports all credential types + via a credential_type discriminator. + """ + + @classmethod + def get_partial_member_list(cls, credential_id: str, credential_type: str) -> Sequence[str]: + """Return account_ids that have partial-member access to a credential.""" + return db.session.scalars( + select(CredentialPermission.account_id).where( + CredentialPermission.credential_id == credential_id, + CredentialPermission.credential_type == credential_type, + ) + ).all() + + @classmethod + def apply_visibility_filter( + cls, + query, + *, + model_id_column: InstrumentedAttribute, + model_user_id_column: InstrumentedAttribute, + model_visibility_column: InstrumentedAttribute, + credential_type: str, + user: Account, + ): + """ + Add WHERE clauses to a SQLAlchemy query so it only returns credentials + visible to the given user. + + - all_team_members: always visible + - only_me: visible only to the creator (user.id matches) + - partial_members: visible to the creator OR users in credential_permissions + - Legacy rows with NULL user_id are treated as all_team_members + - No admin bypass: personal credentials are private regardless of role + """ + # Subquery: credential_ids where user has partial-member permission + partial_subquery = ( + select(CredentialPermission.credential_id) + .where( + CredentialPermission.credential_type == credential_type, + CredentialPermission.account_id == user.id, + ) + .correlate_except(CredentialPermission) + ) + + return query.where( + or_( + # all_team is always visible + model_visibility_column == PermissionEnum.ALL_TEAM, + # legacy rows with NULL user_id treated as all_team + model_user_id_column.is_(None), + # only_me: creator sees their own + (model_user_id_column == user.id), + # partial_members: user is in the permission table + model_id_column.in_(partial_subquery), + ) + ) diff --git a/api/services/datasource_provider_service.py b/api/services/datasource_provider_service.py index e3c6f122ab..12807a41f0 100644 --- a/api/services/datasource_provider_service.py +++ b/api/services/datasource_provider_service.py @@ -1,7 +1,10 @@ import logging import time from collections.abc import Mapping -from typing import Any +from typing import TYPE_CHECKING, Any + +if TYPE_CHECKING: + from models.account import Account from sqlalchemy import delete, func, select, update from sqlalchemy.orm import Session, sessionmaker @@ -791,24 +794,42 @@ class DatasourceProviderService: return secret_input_form_variables - def list_datasource_credentials(self, tenant_id: str, provider: str, plugin_id: str) -> list[dict]: + def list_datasource_credentials( + self, + tenant_id: str, + provider: str, + plugin_id: str, + user: "Account | None" = None, + ) -> list[dict]: """ - list datasource credentials with obfuscated sensitive fields. + list datasource credentials with obfuscated sensitive fields, + filtered by visibility. :param tenant_id: workspace id - :param provider_id: provider id + :param provider: provider name + :param plugin_id: plugin id + :param user: current user (id + admin flag drive the visibility filter) :return: """ + from models.credential_permission import CredentialType as CredPermType + from services.credential_permission_service import CredentialPermissionService + # Get all provider configurations of the current workspace - datasource_providers: list[DatasourceProvider] = list( - db.session.scalars( - select(DatasourceProvider).where( - DatasourceProvider.tenant_id == tenant_id, - DatasourceProvider.provider == provider, - DatasourceProvider.plugin_id == plugin_id, - ) - ).all() + query = select(DatasourceProvider).where( + DatasourceProvider.tenant_id == tenant_id, + DatasourceProvider.provider == provider, + DatasourceProvider.plugin_id == plugin_id, ) + if user is not None: + query = CredentialPermissionService.apply_visibility_filter( + query, + model_id_column=DatasourceProvider.id, + model_user_id_column=DatasourceProvider.user_id, + model_visibility_column=DatasourceProvider.visibility, + credential_type=CredPermType.DATASOURCE_PROVIDER, + user=user, + ) + datasource_providers: list[DatasourceProvider] = list(db.session.scalars(query).all()) if not datasource_providers: return [] copy_credentials_list = [] diff --git a/api/services/model_provider_service.py b/api/services/model_provider_service.py index 51cda79661..362aa6103d 100644 --- a/api/services/model_provider_service.py +++ b/api/services/model_provider_service.py @@ -1,5 +1,8 @@ import logging -from typing import Any +from typing import TYPE_CHECKING, Any + +if TYPE_CHECKING: + from models.account import Account from core.entities.model_entities import ModelWithProviderEntity, ProviderModelWithStatusEntity from core.plugin.impl.model_runtime_factory import create_plugin_model_provider_factory, create_plugin_provider_manager @@ -148,10 +151,11 @@ class ModelProviderService: for model in provider_configurations.get_models(provider=provider) ] - def get_provider_available_credentials(self, tenant_id: str, provider: str): + def get_provider_available_credentials(self, tenant_id: str, provider: str, user: "Account | None" = None): return self._get_provider_manager(tenant_id).get_provider_available_credentials( tenant_id=tenant_id, provider_name=provider, + user=user, ) def get_provider_model_available_credentials( diff --git a/api/services/tools/builtin_tools_manage_service.py b/api/services/tools/builtin_tools_manage_service.py index 6b7092e318..22086317c0 100644 --- a/api/services/tools/builtin_tools_manage_service.py +++ b/api/services/tools/builtin_tools_manage_service.py @@ -30,6 +30,7 @@ from core.tools.utils.encryption import create_provider_encrypter from core.tools.utils.system_encryption import decrypt_system_params from extensions.ext_database import db from extensions.ext_redis import redis_client +from models.account import Account from models.provider_ids import ToolProviderID from models.tools import BuiltinToolProvider, ToolOAuthSystemClient, ToolOAuthTenantClient from services.tools.tools_transform_service import ToolTransformService @@ -206,6 +207,9 @@ class BuiltinToolManageService: db_provider.name = name + # Visibility is immutable after creation — no update path. To change scope, + # create a new credential. partial-member access is handled by RBAC. + except Exception as e: raise ValueError(str(e)) return {"result": "success"} @@ -219,6 +223,7 @@ class BuiltinToolManageService: credentials: dict[str, Any], expires_at: int = -1, name: str | None = None, + visibility: str | None = None, ): """ add builtin tool provider @@ -277,6 +282,14 @@ class BuiltinToolManageService: cache=NoOpProviderCredentialCache(), ) + from models.enums import PermissionEnum + + visibility_enum = PermissionEnum(visibility) if visibility else PermissionEnum.ALL_TEAM + # Plugin credentials only expose only_me / all_team_members at creation; + # partial-member access is handled by workspace RBAC, not per-credential. + if visibility_enum == PermissionEnum.PARTIAL_TEAM: + raise ValueError("partial_members visibility is no longer supported for plugin credentials") + db_provider = BuiltinToolProvider( tenant_id=tenant_id, user_id=user_id, @@ -285,9 +298,11 @@ class BuiltinToolManageService: credential_type=api_type, name=name, expires_at=expires_at if expires_at is not None else -1, + visibility=visibility_enum, ) session.add(db_provider) + session.flush() except Exception as e: raise ValueError(str(e)) @@ -330,24 +345,69 @@ class BuiltinToolManageService: @staticmethod def get_builtin_tool_provider_credentials( - tenant_id: str, provider_name: str + tenant_id: str, + provider_name: str, + user: Account | None = None, + include_credential_ids: list[str] | None = None, ) -> list[ToolProviderCredentialApiEntity]: """ - get builtin tool provider credentials - """ - with db.session.no_autoflush: - providers = db.session.scalars( - select(BuiltinToolProvider) - .where(BuiltinToolProvider.tenant_id == tenant_id, BuiltinToolProvider.provider == provider_name) - .order_by(BuiltinToolProvider.is_default.desc(), BuiltinToolProvider.created_at.asc()) - ).all() + get builtin tool provider credentials, filtered by visibility. - if len(providers) == 0: + ``user`` is used to filter the result list by per-credential visibility + (only_me / all_team_members / legacy partial_members). When ``None`` the + query returns every credential for the tenant — meant for internal / + background callers that don't act on behalf of a specific user. + + ``include_credential_ids`` lets callers request specific credential IDs that should be + returned even if the visibility filter would normally hide them (e.g. an only_me credential + owned by another member which the current workflow/agent node still references). Those + rows are marked with ``from_other_member=True`` so the UI can render them as + borrowed-from-teammate (selectable but not editable). + """ + from models.credential_permission import CredentialType as CredPermType + from services.credential_permission_service import CredentialPermissionService + + with db.session.no_autoflush: + base_filter = ( + BuiltinToolProvider.tenant_id == tenant_id, + BuiltinToolProvider.provider == provider_name, + ) + order = (BuiltinToolProvider.is_default.desc(), BuiltinToolProvider.created_at.asc()) + visible_query = select(BuiltinToolProvider).where(*base_filter).order_by(*order) + if user is not None: + visible_query = CredentialPermissionService.apply_visibility_filter( + visible_query, + model_id_column=BuiltinToolProvider.id, + model_user_id_column=BuiltinToolProvider.user_id, + model_visibility_column=BuiltinToolProvider.visibility, + credential_type=CredPermType.BUILTIN_TOOL_PROVIDER, + user=user, + ) + visible_providers = list(db.session.scalars(visible_query).all()) + + # Fetch any explicitly-included IDs that the visibility filter excluded. + borrowed_ids: set[str] = set() + borrowed_providers: list[BuiltinToolProvider] = [] + if include_credential_ids: + visible_id_set = {p.id for p in visible_providers} + wanted_ids = [cid for cid in include_credential_ids if cid and cid not in visible_id_set] + if wanted_ids: + borrowed_query = ( + select(BuiltinToolProvider) + .where(*base_filter, BuiltinToolProvider.id.in_(wanted_ids)) + .order_by(*order) + ) + borrowed_providers = list(db.session.scalars(borrowed_query).all()) + borrowed_ids = {p.id for p in borrowed_providers} + + providers = visible_providers + borrowed_providers + if not providers: return [] - default_provider = providers[0] - default_provider.is_default = True - provider_controller = ToolManager.get_builtin_provider(default_provider.provider, tenant_id) + # Only the first visible row should be flagged is_default in the response. + if visible_providers: + visible_providers[0].is_default = True + provider_controller = ToolManager.get_builtin_provider(providers[0].provider, tenant_id) credentials: list[ToolProviderCredentialApiEntity] = [] for provider in providers: @@ -359,17 +419,40 @@ class BuiltinToolManageService: provider=provider, credentials=dict(decrypt_credential), ) + # Attach visibility, creator, and partial member list to the response entity + vis = getattr(provider, "visibility", "all_team_members") + vis_str = vis.value if hasattr(vis, "value") else str(vis) + credential_entity.visibility = vis_str + credential_entity.created_by = getattr(provider, "user_id", "") or "" + if vis_str == "partial_members": + credential_entity.partial_member_list = list( + CredentialPermissionService.get_partial_member_list( + provider.id, CredPermType.BUILTIN_TOOL_PROVIDER + ) + ) + if provider.id in borrowed_ids: + credential_entity.from_other_member = True credentials.append(credential_entity) return credentials @staticmethod - def get_builtin_tool_provider_credential_info(tenant_id: str, provider: str) -> ToolProviderCredentialInfoApiEntity: + def get_builtin_tool_provider_credential_info( + tenant_id: str, + provider: str, + user: Account | None = None, + include_credential_ids: list[str] | None = None, + ) -> ToolProviderCredentialInfoApiEntity: """ get builtin tool provider credential info """ provider_controller = ToolManager.get_builtin_provider(provider, tenant_id) supported_credential_types = provider_controller.get_supported_credential_types() - credentials = BuiltinToolManageService.get_builtin_tool_provider_credentials(tenant_id, provider) + credentials = BuiltinToolManageService.get_builtin_tool_provider_credentials( + tenant_id, + provider, + user=user, + include_credential_ids=include_credential_ids, + ) credential_info = ToolProviderCredentialInfoApiEntity( supported_credential_types=supported_credential_types, is_oauth_custom_client_enabled=BuiltinToolManageService.is_oauth_custom_client_enabled(tenant_id, provider), diff --git a/api/services/trigger/trigger_provider_service.py b/api/services/trigger/trigger_provider_service.py index 855f88fb90..005bcd40a8 100644 --- a/api/services/trigger/trigger_provider_service.py +++ b/api/services/trigger/trigger_provider_service.py @@ -3,7 +3,10 @@ import logging import time as _time import uuid from collections.abc import Mapping -from typing import Any, TypedDict +from typing import TYPE_CHECKING, Any, TypedDict + +if TYPE_CHECKING: + from models.account import Account from sqlalchemy import delete, desc, func, select from sqlalchemy.orm import Session, sessionmaker @@ -66,21 +69,37 @@ class TriggerProviderService: @classmethod def list_trigger_provider_subscriptions( - cls, tenant_id: str, provider_id: TriggerProviderID + cls, + tenant_id: str, + provider_id: TriggerProviderID, + user: "Account | None" = None, ) -> list[TriggerProviderSubscriptionApiEntity]: - """List all trigger subscriptions for the current tenant""" + """List all trigger subscriptions for the current tenant, filtered by visibility.""" + from models.credential_permission import CredentialType as CredPermType + from services.credential_permission_service import CredentialPermissionService + subscriptions: list[TriggerProviderSubscriptionApiEntity] = [] workflows_in_use_map: dict[str, int] = {} with Session(db.engine, expire_on_commit=False) as session: - # Get all subscriptions - subscriptions_db = session.scalars( + # Get all subscriptions with visibility filtering + query = ( select(TriggerSubscription) .where( TriggerSubscription.tenant_id == tenant_id, TriggerSubscription.provider_id == str(provider_id), ) .order_by(desc(TriggerSubscription.created_at)) - ).all() + ) + if user is not None: + query = CredentialPermissionService.apply_visibility_filter( + query, + model_id_column=TriggerSubscription.id, + model_user_id_column=TriggerSubscription.user_id, + model_visibility_column=TriggerSubscription.visibility, + credential_type=CredPermType.TRIGGER_SUBSCRIPTION, + user=user, + ) + subscriptions_db = session.scalars(query).all() subscriptions = [subscription.to_api_entity() for subscription in subscriptions_db] if not subscriptions: return [] diff --git a/api/tests/test_containers_integration_tests/controllers/console/workspace/test_tool_provider.py b/api/tests/test_containers_integration_tests/controllers/console/workspace/test_tool_provider.py index b977a3eb7a..d5eb53ad99 100644 --- a/api/tests/test_containers_integration_tests/controllers/console/workspace/test_tool_provider.py +++ b/api/tests/test_containers_integration_tests/controllers/console/workspace/test_tool_provider.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +from types import SimpleNamespace from unittest.mock import MagicMock, patch import pytest @@ -284,11 +285,12 @@ class TestBuiltinProviderApis: api = ToolBuiltinProviderGetCredentialsApi() method = unwrap(api.get) + mock_user = SimpleNamespace(id="user-1", is_admin_or_owner=False) with ( app.test_request_context("/"), patch( "controllers.console.workspace.tool_providers.current_account_with_tenant", - return_value=(None, "t"), + return_value=(mock_user, "t"), ), patch( "controllers.console.workspace.tool_providers.BuiltinToolManageService.get_builtin_tool_provider_credentials", diff --git a/api/tests/unit_tests/controllers/console/workspace/test_tool_providers.py b/api/tests/unit_tests/controllers/console/workspace/test_tool_providers.py index 1422f29849..51b21a2ef0 100644 --- a/api/tests/unit_tests/controllers/console/workspace/test_tool_providers.py +++ b/api/tests/unit_tests/controllers/console/workspace/test_tool_providers.py @@ -81,7 +81,13 @@ def controller_module(monkeypatch: pytest.MonkeyPatch): def _mock_account(user_id: str = "user-123") -> SimpleNamespace: - return SimpleNamespace(id=user_id, status="active", is_authenticated=True, current_tenant_id=None) + return SimpleNamespace( + id=user_id, + status="active", + is_authenticated=True, + current_tenant_id=None, + is_admin_or_owner=False, + ) def _set_current_account( @@ -149,6 +155,7 @@ def test_builtin_provider_add_passes_payload( credentials={"api_key": "sk-test"}, name="MyTool", api_type=controller_module.CredentialType.API_KEY, + visibility=None, ) @@ -197,7 +204,12 @@ def test_builtin_provider_credentials_get(app: Flask, controller_module, monkeyp resp = controller_module.ToolBuiltinProviderGetCredentialsApi().get(provider="demo") assert resp == [{"cred": 1}] - service_mock.assert_called_once_with(tenant_id="tenant-cred", provider_name="demo") + service_mock.assert_called_once_with( + tenant_id="tenant-cred", + provider_name="demo", + user=user, + include_credential_ids=None, + ) def test_api_provider_remote_schema_get(app: Flask, controller_module, monkeypatch: pytest.MonkeyPatch): diff --git a/api/tests/unit_tests/services/test_credential_permission_service.py b/api/tests/unit_tests/services/test_credential_permission_service.py new file mode 100644 index 0000000000..687d688017 --- /dev/null +++ b/api/tests/unit_tests/services/test_credential_permission_service.py @@ -0,0 +1,103 @@ +"""Unit tests for CredentialPermissionService. + +Tests the visibility filtering logic, partial-member read path, +and admin bypass behavior. +""" + +from types import SimpleNamespace +from unittest.mock import MagicMock, patch +from uuid import uuid4 + +import pytest +from sqlalchemy import select + +from models.credential_permission import CredentialType +from services.credential_permission_service import CredentialPermissionService + + +@pytest.fixture +def tenant_id(): + return str(uuid4()) + + +@pytest.fixture +def user_id(): + return str(uuid4()) + + +@pytest.fixture +def other_user_id(): + return str(uuid4()) + + +@pytest.fixture +def credential_id(): + return str(uuid4()) + + +class TestGetPartialMemberList: + def test_returns_empty_when_no_permissions(self, credential_id): + with patch("services.credential_permission_service.db") as mock_db: + mock_db.session.scalars.return_value.all.return_value = [] + result = CredentialPermissionService.get_partial_member_list( + credential_id, CredentialType.TRIGGER_SUBSCRIPTION + ) + assert result == [] + + def test_returns_account_ids(self, credential_id, user_id, other_user_id): + with patch("services.credential_permission_service.db") as mock_db: + mock_db.session.scalars.return_value.all.return_value = [user_id, other_user_id] + result = CredentialPermissionService.get_partial_member_list( + credential_id, CredentialType.TRIGGER_SUBSCRIPTION + ) + assert set(result) == {user_id, other_user_id} + + +class TestApplyVisibilityFilter: + """Test the visibility filter logic using mock model columns.""" + + def _make_mock_columns(self): + """Create mock model columns for testing.""" + model_id = MagicMock(name="id_column") + model_user_id = MagicMock(name="user_id_column") + model_visibility = MagicMock(name="visibility_column") + return model_id, model_user_id, model_visibility + + def _make_user(self, user_id: str, is_admin: bool): + return SimpleNamespace(id=user_id, is_admin_or_owner=is_admin) + + def test_admin_gets_filtered_too(self, user_id): + """Admin should NOT bypass visibility — personal credentials are private regardless of role.""" + from models.trigger import TriggerSubscription + + query = select(TriggerSubscription) + result = CredentialPermissionService.apply_visibility_filter( + query, + model_id_column=TriggerSubscription.id, + model_user_id_column=TriggerSubscription.user_id, + model_visibility_column=TriggerSubscription.visibility, + credential_type=CredentialType.TRIGGER_SUBSCRIPTION, + user=self._make_user(user_id, is_admin=True), + ) + # No admin bypass: query should have WHERE clause + compiled = str(result.compile(compile_kwargs={"literal_binds": True})) + assert "WHERE" in compiled + + def test_non_admin_adds_filter_on_real_model(self, user_id): + """Non-admin should get a filtered query when using real SQLAlchemy columns.""" + from models.trigger import TriggerSubscription + + query = select(TriggerSubscription) + result = CredentialPermissionService.apply_visibility_filter( + query, + model_id_column=TriggerSubscription.id, + model_user_id_column=TriggerSubscription.user_id, + model_visibility_column=TriggerSubscription.visibility, + credential_type=CredentialType.TRIGGER_SUBSCRIPTION, + user=self._make_user(user_id, is_admin=False), + ) + # The compiled SQL should include a WHERE clause referencing user_id and visibility + compiled = str(result.compile(compile_kwargs={"literal_binds": True})) + assert "WHERE" in compiled + assert "visibility" in compiled + assert "user_id" in compiled diff --git a/eslint-suppressions.json b/eslint-suppressions.json index 39dca866fe..440a37dd3f 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -1598,6 +1598,11 @@ "count": 1 } }, + "web/app/components/base/permission-selector/index.tsx": { + "no-restricted-imports": { + "count": 1 + } + }, "web/app/components/base/prompt-editor/index.stories.tsx": { "no-console": { "count": 1 @@ -2715,9 +2720,6 @@ "web/app/components/plugins/plugin-auth/authorized/item.tsx": { "no-restricted-imports": { "count": 1 - }, - "ts/no-explicit-any": { - "count": 1 } }, "web/app/components/plugins/plugin-auth/hooks/use-get-api.ts": { @@ -2739,9 +2741,6 @@ }, "no-barrel-files/no-barrel-files": { "count": 2 - }, - "ts/no-explicit-any": { - "count": 1 } }, "web/app/components/plugins/plugin-auth/utils.ts": { @@ -5152,7 +5151,7 @@ }, "web/models/datasets.ts": { "erasable-syntax-only/enums": { - "count": 8 + "count": 7 }, "ts/no-explicit-any": { "count": 5 diff --git a/packages/contracts/generated/api/console/datasets/types.gen.ts b/packages/contracts/generated/api/console/datasets/types.gen.ts index b92f0b8754..cb7a869214 100644 --- a/packages/contracts/generated/api/console/datasets/types.gen.ts +++ b/packages/contracts/generated/api/console/datasets/types.gen.ts @@ -18,7 +18,7 @@ export type DatasetCreatePayload = { external_knowledge_id?: string | null indexing_technique?: string | null name: string - permission?: DatasetPermissionEnum + permission?: PermissionEnum provider?: string } @@ -272,7 +272,7 @@ export type DatasetUpdatePayload = { partial_member_list?: Array<{ [key: string]: string }> | null - permission?: DatasetPermissionEnum + permission?: PermissionEnum retrieval_model?: { [key: string]: unknown } | null @@ -544,7 +544,7 @@ export type DatasetListItemResponse = { word_count: number } -export type DatasetPermissionEnum = 'all_team_members' | 'only_me' | 'partial_members' +export type PermissionEnum = 'all_team_members' | 'only_me' | 'partial_members' export type DatasetDocMetadataResponse = { id: string diff --git a/packages/contracts/generated/api/console/datasets/zod.gen.ts b/packages/contracts/generated/api/console/datasets/zod.gen.ts index 6539cd79d8..87361f07c8 100644 --- a/packages/contracts/generated/api/console/datasets/zod.gen.ts +++ b/packages/contracts/generated/api/console/datasets/zod.gen.ts @@ -290,9 +290,11 @@ export const zUsageCheckResponse = z.object({ }) /** - * DatasetPermissionEnum + * PermissionEnum + * + * Shared permission levels for resources (datasets, credentials, etc.) */ -export const zDatasetPermissionEnum = z.enum(['all_team_members', 'only_me', 'partial_members']) +export const zPermissionEnum = z.enum(['all_team_members', 'only_me', 'partial_members']) /** * DatasetCreatePayload @@ -303,7 +305,7 @@ export const zDatasetCreatePayload = z.object({ external_knowledge_id: z.string().nullish(), indexing_technique: z.string().nullish(), name: z.string().min(1).max(40), - permission: zDatasetPermissionEnum.optional(), + permission: zPermissionEnum.optional(), provider: z.string().optional().default('vendor'), }) @@ -322,7 +324,7 @@ export const zDatasetUpdatePayload = z.object({ is_multimodal: z.boolean().nullish().default(false), name: z.string().min(1).max(40).nullish(), partial_member_list: z.array(z.record(z.string(), z.string())).nullish(), - permission: zDatasetPermissionEnum.optional(), + permission: zPermissionEnum.optional(), retrieval_model: z.record(z.string(), z.unknown()).nullish(), summary_index_setting: z.record(z.string(), z.unknown()).nullish(), }) diff --git a/packages/contracts/generated/api/console/workspaces/types.gen.ts b/packages/contracts/generated/api/console/workspaces/types.gen.ts index e943f7924a..8336951b51 100644 --- a/packages/contracts/generated/api/console/workspaces/types.gen.ts +++ b/packages/contracts/generated/api/console/workspaces/types.gen.ts @@ -354,6 +354,7 @@ export type BuiltinToolAddPayload = { } name?: string | null type: CredentialType + visibility?: string | null } export type BuiltinProviderDefaultCredentialPayload = { diff --git a/packages/contracts/generated/api/console/workspaces/zod.gen.ts b/packages/contracts/generated/api/console/workspaces/zod.gen.ts index 5c342d7d27..a6f133a349 100644 --- a/packages/contracts/generated/api/console/workspaces/zod.gen.ts +++ b/packages/contracts/generated/api/console/workspaces/zod.gen.ts @@ -704,6 +704,7 @@ export const zBuiltinToolAddPayload = z.object({ credentials: z.record(z.string(), z.unknown()), name: z.string().max(30).nullish(), type: zCredentialType, + visibility: z.string().nullish(), }) /** diff --git a/packages/contracts/generated/api/service/types.gen.ts b/packages/contracts/generated/api/service/types.gen.ts index 771eb34d3a..381103b0d5 100644 --- a/packages/contracts/generated/api/service/types.gen.ts +++ b/packages/contracts/generated/api/service/types.gen.ts @@ -188,7 +188,7 @@ export type DatasetCreatePayload = { external_knowledge_id?: string | null indexing_technique?: 'economy' | 'high_quality' | null name: string - permission?: DatasetPermissionEnum + permission?: PermissionEnum provider?: string retrieval_model?: RetrievalModel summary_index_setting?: { @@ -350,8 +350,6 @@ export type DatasetMetadataResponse = { type: string } -export type DatasetPermissionEnum = 'all_team_members' | 'only_me' | 'partial_members' - export type DatasetRerankingModelResponse = { reranking_model_name?: string | null reranking_provider_name?: string | null @@ -395,7 +393,7 @@ export type DatasetUpdatePayload = { partial_member_list?: Array<{ [key: string]: string }> | null - permission?: DatasetPermissionEnum + permission?: PermissionEnum retrieval_model?: RetrievalModel } @@ -700,6 +698,8 @@ export type MetadataUpdatePayload = { name: string } +export type PermissionEnum = 'all_team_members' | 'only_me' | 'partial_members' + export type PipelineRunApiEntity = { datasource_info_list: Array<{ [key: string]: unknown diff --git a/packages/contracts/generated/api/service/zod.gen.ts b/packages/contracts/generated/api/service/zod.gen.ts index 194ee70e60..378d368b12 100644 --- a/packages/contracts/generated/api/service/zod.gen.ts +++ b/packages/contracts/generated/api/service/zod.gen.ts @@ -350,11 +350,6 @@ export const zDatasetMetadataResponse = z.object({ type: z.string(), }) -/** - * DatasetPermissionEnum - */ -export const zDatasetPermissionEnum = z.enum(['all_team_members', 'only_me', 'partial_members']) - /** * DatasetRerankingModelResponse */ @@ -870,6 +865,13 @@ export const zMetadataUpdatePayload = z.object({ name: z.string(), }) +/** + * PermissionEnum + * + * Shared permission levels for resources (datasets, credentials, etc.) + */ +export const zPermissionEnum = z.enum(['all_team_members', 'only_me', 'partial_members']) + /** * PipelineRunApiEntity */ @@ -1225,7 +1227,7 @@ export const zDatasetCreatePayload = z.object({ external_knowledge_id: z.string().nullish(), indexing_technique: z.enum(['economy', 'high_quality']).nullish(), name: z.string().min(1).max(40), - permission: zDatasetPermissionEnum.optional(), + permission: zPermissionEnum.optional(), provider: z.string().optional().default('vendor'), retrieval_model: zRetrievalModel.optional(), summary_index_setting: z.record(z.string(), z.unknown()).nullish(), @@ -1244,7 +1246,7 @@ export const zDatasetUpdatePayload = z.object({ indexing_technique: z.enum(['economy', 'high_quality']).nullish(), name: z.string().min(1).max(40).nullish(), partial_member_list: z.array(z.record(z.string(), z.string())).nullish(), - permission: zDatasetPermissionEnum.optional(), + permission: zPermissionEnum.optional(), retrieval_model: zRetrievalModel.optional(), }) diff --git a/web/app/components/base/permission-selector/index.tsx b/web/app/components/base/permission-selector/index.tsx new file mode 100644 index 0000000000..fd4fca4e7e --- /dev/null +++ b/web/app/components/base/permission-selector/index.tsx @@ -0,0 +1,279 @@ +import type { Member } from '@/models/common' +import { Avatar } from '@langgenius/dify-ui/avatar' +import { cn } from '@langgenius/dify-ui/cn' +import { + Popover, + PopoverContent, + PopoverTrigger, +} from '@langgenius/dify-ui/popover' +import { RiArrowDownSLine, RiGroup2Line, RiLock2Line } from '@remixicon/react' +import { useDebounceFn } from 'ahooks' +import * as React from 'react' +import { useCallback, useMemo, useState } from 'react' +import { useTranslation } from 'react-i18next' +import Input from '@/app/components/base/input' +import { useSelector as useAppContextWithSelector } from '@/context/app-context' +import { PermissionLevel } from '@/models/permission' +import MemberItem from './member-item' +import Item from './permission-item' + +type PermissionSelectorProps = { + disabled?: boolean + permission?: PermissionLevel + value: string[] + memberList: Member[] + onChange: (permission?: PermissionLevel) => void + onMemberSelect: (v: string[]) => void + /** i18n namespace for label strings (defaults to datasetSettings for backward compat) */ + i18nNamespace?: string + /** + * Hide the "Partial members" option. Useful for surfaces (e.g. plugin + * credential creation) where partial-member access is delegated to RBAC + * and the picker should only expose only_me / all_team_members. + */ + hidePartialMembers?: boolean +} + +const PermissionSelector = ({ + disabled, + permission, + value, + memberList, + onChange, + onMemberSelect, + i18nNamespace = 'datasetSettings', + hidePartialMembers = false, +}: PermissionSelectorProps) => { + const { t } = useTranslation() + const userProfile = useAppContextWithSelector(state => state.userProfile) + const [open, setOpen] = useState(false) + + const [keywords, setKeywords] = useState('') + const [searchKeywords, setSearchKeywords] = useState('') + const { run: handleSearch } = useDebounceFn(() => { + setSearchKeywords(keywords) + }, { wait: 500 }) + const handleKeywordsChange = (value: string) => { + setKeywords(value) + handleSearch() + } + const selectMember = useCallback((member: Member) => { + if (value.includes(member.id)) + onMemberSelect(value.filter(v => v !== member.id)) + else + onMemberSelect([...value, member.id]) + }, [value, onMemberSelect]) + + const selectedMembers = useMemo(() => { + return [ + userProfile, + ...memberList.filter(member => member.id !== userProfile.id).filter(member => value.includes(member.id)), + ] + }, [userProfile, value, memberList]) + + const showMe = useMemo(() => { + return userProfile.name.includes(searchKeywords) || userProfile.email.includes(searchKeywords) + }, [searchKeywords, userProfile]) + + const filteredMemberList = useMemo(() => { + return memberList.filter(member => (member.name.includes(searchKeywords) || member.email.includes(searchKeywords)) && member.id !== userProfile.id && ['owner', 'admin', 'editor', 'dataset_operator'].includes(member.role)) + }, [memberList, searchKeywords, userProfile]) + + const onSelectOnlyMe = useCallback(() => { + onChange(PermissionLevel.onlyMe) + setOpen(false) + }, [onChange]) + + const onSelectAllMembers = useCallback(() => { + onChange(PermissionLevel.allTeamMembers) + setOpen(false) + }, [onChange]) + + const onSelectPartialMembers = useCallback(() => { + onChange(PermissionLevel.partialMembers) + onMemberSelect([userProfile.id]) + }, [onChange, onMemberSelect, userProfile]) + + const isOnlyMe = permission === PermissionLevel.onlyMe + const isAllTeamMembers = permission === PermissionLevel.allTeamMembers + const isPartialMembers = permission === PermissionLevel.partialMembers + const selectedMemberNames = selectedMembers.map(member => member.name).join(', ') + + return ( + +
+ + )} + > + { + isOnlyMe && ( + <> +
+ +
+
+ {t('form.permissionsOnlyMe', { ns: i18nNamespace })} +
+ + ) + } + { + isAllTeamMembers && ( + <> +
+ +
+
+ {t('form.permissionsAllMember', { ns: i18nNamespace })} +
+ + ) + } + { + isPartialMembers && ( + <> +
+ { + selectedMembers.length === 1 && ( + + ) + } + { + selectedMembers.length >= 2 && ( + <> + + + + ) + } +
+
+ {selectedMemberNames} +
+ + ) + } + + + +
+ {/* Only me */} + + } + text={t('form.permissionsOnlyMe', { ns: i18nNamespace })} + onClick={onSelectOnlyMe} + isSelected={isOnlyMe} + /> + {/* All team members */} + + +
+ )} + text={t('form.permissionsAllMember', { ns: i18nNamespace })} + onClick={onSelectAllMembers} + isSelected={isAllTeamMembers} + /> + {/* Partial members */} + {!hidePartialMembers && ( + + +
+ )} + text={t('form.permissionsInvitedMembers', { ns: i18nNamespace })} + onClick={onSelectPartialMembers} + isSelected={isPartialMembers} + /> + )} + + {!hidePartialMembers && isPartialMembers && ( +
+
+ handleKeywordsChange(e.target.value)} + onClear={() => handleKeywordsChange('')} + /> +
+
+ {showMe && ( + + } + name={userProfile.name} + email={userProfile.email} + isSelected + isMe + i18nNamespace={i18nNamespace} + /> + )} + {filteredMemberList.map(member => ( + + } + name={member.name} + email={member.email} + isSelected={value.includes(member.id)} + onClick={selectMember.bind(null, member)} + i18nNamespace={i18nNamespace} + /> + ))} + { + !showMe && filteredMemberList.length === 0 && ( +
+ {t('form.onSearchResults', { ns: i18nNamespace })} +
+ ) + } +
+
+ )} + + + + ) +} + +export default PermissionSelector diff --git a/web/app/components/base/permission-selector/member-item.tsx b/web/app/components/base/permission-selector/member-item.tsx new file mode 100644 index 0000000000..01826c64da --- /dev/null +++ b/web/app/components/base/permission-selector/member-item.tsx @@ -0,0 +1,49 @@ +import { cn } from '@langgenius/dify-ui/cn' +import { RiCheckLine } from '@remixicon/react' +import * as React from 'react' +import { useTranslation } from 'react-i18next' + +type MemberItemProps = { + leftIcon: React.ReactNode + name: string + email: string + isSelected: boolean + isMe?: boolean + onClick?: () => void + i18nNamespace?: string +} + +const MemberItem = ({ + leftIcon, + name, + email, + isSelected, + isMe = false, + onClick, + i18nNamespace = 'datasetSettings', +}: MemberItemProps) => { + const { t } = useTranslation() + + return ( +
+ {leftIcon} +
+
+ {name} + {isMe && ( + + {t('form.me', { ns: i18nNamespace })} + + )} +
+
{email}
+
+ {isSelected && } +
+ ) +} + +export default React.memo(MemberItem) diff --git a/web/app/components/base/permission-selector/permission-item.tsx b/web/app/components/base/permission-selector/permission-item.tsx new file mode 100644 index 0000000000..28e1c78be2 --- /dev/null +++ b/web/app/components/base/permission-selector/permission-item.tsx @@ -0,0 +1,31 @@ +import { RiCheckLine } from '@remixicon/react' +import * as React from 'react' + +type PermissionItemProps = { + leftIcon: React.ReactNode + text: string + onClick: () => void + isSelected: boolean +} + +const PermissionItem = ({ + leftIcon, + text, + onClick, + isSelected, +}: PermissionItemProps) => { + return ( +
+ {leftIcon} +
+ {text} +
+ {isSelected && } +
+ ) +} + +export default React.memo(PermissionItem) diff --git a/web/app/components/plugins/plugin-auth/__tests__/authorized-in-node.spec.tsx b/web/app/components/plugins/plugin-auth/__tests__/authorized-in-node.spec.tsx index cba8dc96c9..eaf586125c 100644 --- a/web/app/components/plugins/plugin-auth/__tests__/authorized-in-node.spec.tsx +++ b/web/app/components/plugins/plugin-auth/__tests__/authorized-in-node.spec.tsx @@ -36,10 +36,14 @@ vi.mock('@/service/use-tools', () => ({ })) const mockIsCurrentWorkspaceManager = vi.fn() +const mockUserProfile = { id: 'test-user', name: 'Test User', email: 'test@example.com', avatar_url: '' } vi.mock('@/context/app-context', () => ({ useAppContext: () => ({ isCurrentWorkspaceManager: mockIsCurrentWorkspaceManager(), }), + // Item renders useAppContextWithSelector(state => state.userProfile) + useSelector: (selector: (state: { userProfile: typeof mockUserProfile }) => unknown) => + selector({ userProfile: mockUserProfile }), })) vi.mock('@/hooks/use-oauth', () => ({ diff --git a/web/app/components/plugins/plugin-auth/__tests__/plugin-auth-in-agent.spec.tsx b/web/app/components/plugins/plugin-auth/__tests__/plugin-auth-in-agent.spec.tsx index 6afe1ae334..d19c05efe1 100644 --- a/web/app/components/plugins/plugin-auth/__tests__/plugin-auth-in-agent.spec.tsx +++ b/web/app/components/plugins/plugin-auth/__tests__/plugin-auth-in-agent.spec.tsx @@ -36,10 +36,15 @@ vi.mock('@/service/use-tools', () => ({ })) const mockIsCurrentWorkspaceManager = vi.fn() +const mockUserProfile = { id: 'test-user', name: 'Test User', email: 'test@example.com', avatar_url: '' } vi.mock('@/context/app-context', () => ({ useAppContext: () => ({ isCurrentWorkspaceManager: mockIsCurrentWorkspaceManager(), }), + // Item renders useAppContextWithSelector(state => state.userProfile) for the + // borrowed-row heuristic. Provide a minimal stub so the selector runs. + useSelector: (selector: (state: { userProfile: typeof mockUserProfile }) => unknown) => + selector({ userProfile: mockUserProfile }), })) vi.mock('@/hooks/use-oauth', () => ({ diff --git a/web/app/components/plugins/plugin-auth/authorize/__tests__/api-key-modal.spec.tsx b/web/app/components/plugins/plugin-auth/authorize/__tests__/api-key-modal.spec.tsx index 6987d6435b..f9b382ef4f 100644 --- a/web/app/components/plugins/plugin-auth/authorize/__tests__/api-key-modal.spec.tsx +++ b/web/app/components/plugins/plugin-auth/authorize/__tests__/api-key-modal.spec.tsx @@ -77,6 +77,18 @@ vi.mock('@/app/components/base/form/types', () => ({ FormTypeEnum: { textInput: 'text-input' }, })) +// PermissionSelector (rendered for create mode) calls useMembers via TanStack Query. +// Stub it so tests don't need a QueryClientProvider wrapper. +vi.mock('@/service/use-common', () => ({ + useMembers: () => ({ data: { accounts: [] } }), +})) + +// PermissionSelector also reads userProfile from app-context. +vi.mock('@/context/app-context', () => ({ + useSelector: (selector: (state: { userProfile: { id: string, name: string, email: string, avatar_url: string } }) => unknown) => + selector({ userProfile: { id: 'test-user', name: 'Test User', email: 'test@example.com', avatar_url: '' } }), +})) + const basePayload = { category: AuthCategory.tool, provider: 'test-provider', diff --git a/web/app/components/plugins/plugin-auth/authorize/add-api-key-button.tsx b/web/app/components/plugins/plugin-auth/authorize/add-api-key-button.tsx index 38f3f85643..83d1d53696 100644 --- a/web/app/components/plugins/plugin-auth/authorize/add-api-key-button.tsx +++ b/web/app/components/plugins/plugin-auth/authorize/add-api-key-button.tsx @@ -15,6 +15,13 @@ export type AddApiKeyButtonProps = { disabled?: boolean onUpdate?: () => void formSchemas?: FormSchema[] + /** + * If provided, clicking the button calls this callback instead of mounting + * the modal inline. Use this when the button lives inside a Popover that + * would unmount the modal on outside-click; the parent should render the + * ApiKeyModal at a level above the Popover. + */ + onClick?: () => void } const AddApiKeyButton = ({ pluginPayload, @@ -23,25 +30,28 @@ const AddApiKeyButton = ({ disabled, onUpdate, formSchemas = [], + onClick, }: AddApiKeyButtonProps) => { const [isApiKeyModalOpen, setIsApiKeyModalOpen] = useState(false) const [isApiKeyModalMounted, setIsApiKeyModalMounted] = useState(false) + const handleClick = onClick ?? (() => { + setIsApiKeyModalMounted(true) + setIsApiKeyModalOpen(true) + }) return ( <> { - isApiKeyModalMounted && ( + // Only mount the modal here when in uncontrolled mode (no onClick prop). + !onClick && isApiKeyModalMounted && ( ( + (editValues?.__visibility__ as PermissionLevel) ?? PermissionLevel.allTeamMembers, + ) + const [selectedMemberIDs, setSelectedMemberIDs] = useState( + (editValues?.__partial_member_list__ as string[]) ?? [], + ) + // Only need member list when creating (the permission selector is hidden on edit). + const { data: membersData } = useMembers() + const memberList = membersData?.accounts ?? [] const mergedData = useMemo(() => { if (formSchemasFromProps?.length) return formSchemasFromProps @@ -98,10 +110,14 @@ const ApiKeyModal = ({ const { __name__, __credential_id__, + __visibility__, + __partial_member_list__, + __created_by__, ...restValues } = values handleSetDoingAction(true) + // Visibility is settable only at creation. On edit we don't send it. if (editValues) { await updatePluginCredential({ credentials: restValues, @@ -110,10 +126,17 @@ const ApiKeyModal = ({ }) } else { + const permissionPayload = { + visibility: permission, + ...(permission === PermissionLevel.partialMembers + ? { partial_member_list: selectedMemberIDs.map(id => ({ user_id: id })) } + : {}), + } await addPluginCredential({ credentials: restValues, type: CredentialTypeEnum.API_KEY, name: __name__ || '', + ...permissionPayload, }) } toast.success(t('api.actionSuccess', { ns: 'common' })) @@ -125,7 +148,7 @@ const ApiKeyModal = ({ finally { handleSetDoingAction(false) } - }, [addPluginCredential, onClose, onOpenChange, onUpdate, updatePluginCredential, t, editValues, handleSetDoingAction]) + }, [addPluginCredential, onClose, onOpenChange, onUpdate, updatePluginCredential, t, editValues, handleSetDoingAction, permission, selectedMemberIDs]) const isDisabled = disabled || isLoading || doingAction const handleOpenChange = useCallback((nextOpen: boolean) => { @@ -176,6 +199,22 @@ const ApiKeyModal = ({ /> ) } + {!isLoading && !editValues && ( +
+
+ {t('auth.whoCanUse', { ns: 'plugin' })} +
+ setPermission(v)} + onMemberSelect={setSelectedMemberIDs} + hidePartialMembers + /> +
+ )}
diff --git a/web/app/components/plugins/plugin-auth/authorize/index.tsx b/web/app/components/plugins/plugin-auth/authorize/index.tsx index a6e53f977e..925359e519 100644 --- a/web/app/components/plugins/plugin-auth/authorize/index.tsx +++ b/web/app/components/plugins/plugin-auth/authorize/index.tsx @@ -20,6 +20,13 @@ type AuthorizeProps = { disabled?: boolean onUpdate?: () => void notAllowCustomCredential?: boolean + /** + * If provided, the API-key button delegates modal-opening to the parent + * instead of rendering it inline. Used when this Authorize is mounted + * inside a Popover whose outside-click handler would otherwise unmount + * the modal. + */ + onApiKeyClick?: () => void } const Authorize = ({ pluginPayload, @@ -30,6 +37,7 @@ const Authorize = ({ disabled, onUpdate, notAllowCustomCredential, + onApiKeyClick, }: AuthorizeProps) => { const { t } = useTranslation() const oAuthButtonProps: AddOAuthButtonProps = useMemo(() => { @@ -57,14 +65,16 @@ const Authorize = ({ pluginPayload, buttonVariant: 'secondary', buttonText: !canOAuth ? t('auth.useApiAuth', { ns: 'plugin' }) : t('auth.addApi', { ns: 'plugin' }), + onClick: onApiKeyClick, } } return { pluginPayload, buttonText: !canOAuth ? t('auth.useApiAuth', { ns: 'plugin' }) : t('auth.addApi', { ns: 'plugin' }), buttonVariant: !canOAuth ? 'primary' : 'secondary-accent', + onClick: onApiKeyClick, } - }, [canOAuth, theme, pluginPayload, t]) + }, [canOAuth, theme, pluginPayload, t, onApiKeyClick]) const OAuthButton = useMemo(() => { const Item = ( diff --git a/web/app/components/plugins/plugin-auth/authorized-in-node.tsx b/web/app/components/plugins/plugin-auth/authorized-in-node.tsx index 0831fe9a15..82c4119b7a 100644 --- a/web/app/components/plugins/plugin-auth/authorized-in-node.tsx +++ b/web/app/components/plugins/plugin-auth/authorized-in-node.tsx @@ -37,7 +37,7 @@ const AuthorizedInNode = ({ disabled, invalidPluginCredentialInfo, notAllowCustomCredential, - } = usePluginAuth(pluginPayload, true) + } = usePluginAuth(pluginPayload, true, credentialId ? [credentialId] : undefined) const renderTrigger = useCallback((open?: boolean) => { let label = '' let removed = false diff --git a/web/app/components/plugins/plugin-auth/authorized/__tests__/item.spec.tsx b/web/app/components/plugins/plugin-auth/authorized/__tests__/item.spec.tsx index 39ec436ece..22ea9d36bb 100644 --- a/web/app/components/plugins/plugin-auth/authorized/__tests__/item.spec.tsx +++ b/web/app/components/plugins/plugin-auth/authorized/__tests__/item.spec.tsx @@ -1,9 +1,18 @@ import type { Credential } from '../../types' import { cleanup, fireEvent, render, screen } from '@testing-library/react' import { beforeEach, describe, expect, it, vi } from 'vitest' + import { CredentialTypeEnum } from '../../types' import Item from '../item' +// Item uses useAppContextWithSelector(state => state.userProfile) for the +// borrowed-row heuristic; provide a minimal mock so the selector resolves. +const mockUserProfile = { id: 'test-user', name: 'Test User', email: 'test@example.com', avatar_url: '' } +vi.mock('@/context/app-context', () => ({ + useSelector: (selector: (state: { userProfile: typeof mockUserProfile }) => unknown) => + selector({ userProfile: mockUserProfile }), +})) + // ==================== Test Utilities ==================== const createCredential = (overrides: Partial = {}): Credential => ({ @@ -53,15 +62,17 @@ describe('Item Component', () => { render() - expect(screen.getByText('Enterprise')).toBeInTheDocument() + expect(screen.getByText('plugin.auth.enterprise')).toBeInTheDocument() }) - it('should not render enterprise badge when from_enterprise is false', () => { - const credential = createCredential({ from_enterprise: false }) + it('should not render personal/shared badge — the Personal/Shared tag was removed per product feedback', () => { + const credential = createCredential({ from_enterprise: false, visibility: 'only_me' }) render() - expect(screen.queryByText('Enterprise')).not.toBeInTheDocument() + expect(screen.queryByText('plugin.auth.personal')).not.toBeInTheDocument() + expect(screen.queryByText('plugin.auth.shared')).not.toBeInTheDocument() + expect(screen.queryByText('plugin.auth.enterprise')).not.toBeInTheDocument() }) it('should render selected icon when showSelectedIcon is true and credential is selected', () => { diff --git a/web/app/components/plugins/plugin-auth/authorized/index.tsx b/web/app/components/plugins/plugin-auth/authorized/index.tsx index 7e5b99a9de..0d708576eb 100644 --- a/web/app/components/plugins/plugin-auth/authorized/index.tsx +++ b/web/app/components/plugins/plugin-auth/authorized/index.tsx @@ -142,6 +142,13 @@ const Authorized = ({ if (!open) pendingOperationCredentialIdRef.current = null }, []) + // Lifted state for the "+ Add API Key" modal so it isn't unmounted when the + // popover closes due to outside-click detection on the modal's portal. + const [isAddApiKeyOpen, setIsAddApiKeyOpen] = useState(false) + const handleAddApiKeyClick = useCallback(() => { + setMergedIsOpen(false) + setIsAddApiKeyOpen(true) + }, [setMergedIsOpen]) const handleRemove = useCallback(() => { setDeleteCredentialId(pendingOperationCredentialIdRef.current) }, []) @@ -334,6 +341,7 @@ const Authorized = ({ canApiKey={canApiKey} disabled={disabled} onUpdate={onUpdate} + onApiKeyClick={handleAddApiKeyClick} />
@@ -371,6 +379,18 @@ const Authorized = ({ /> ) } + { + isAddApiKeyOpen && ( + setIsAddApiKeyOpen(false)} + disabled={disabled || doingAction} + onUpdate={onUpdate} + /> + ) + } ) } diff --git a/web/app/components/plugins/plugin-auth/authorized/item.tsx b/web/app/components/plugins/plugin-auth/authorized/item.tsx index aae4a481f8..5d91c34e8e 100644 --- a/web/app/components/plugins/plugin-auth/authorized/item.tsx +++ b/web/app/components/plugins/plugin-auth/authorized/item.tsx @@ -8,6 +8,7 @@ import { RiDeleteBinLine, RiEditLine, RiEqualizer2Line, + RiInformationLine, } from '@remixicon/react' import { memo, @@ -18,13 +19,14 @@ import { useTranslation } from 'react-i18next' import ActionButton from '@/app/components/base/action-button' import Badge from '@/app/components/base/badge' import Input from '@/app/components/base/input' +import { useSelector as useAppContextWithSelector } from '@/context/app-context' import { CredentialTypeEnum } from '../types' type ItemProps = { credential: Credential disabled?: boolean onDelete?: (id: string) => void - onEdit?: (id: string, values: Record) => void + onEdit?: (id: string, values: Record) => void onSetDefault?: (id: string) => void onRename?: (payload: { credential_id: string @@ -57,6 +59,18 @@ const Item = ({ const [renaming, setRenaming] = useState(false) const [renameValue, setRenameValue] = useState(credential.name) const isOAuth = credential.credential_type === CredentialTypeEnum.OAUTH2 + const isPersonal = credential.visibility === 'only_me' + const userProfile = useAppContextWithSelector(state => state.userProfile) + // Borrowed-from-teammate: the backend explicitly flagged this row as another member's + // only_me credential, returned only because the current node still references it. + // Fallback heuristic (created_by mismatch on a selected row) is kept for backends + // that don't yet emit the flag. + const isSelected = showSelectedIcon && selectedCredentialId === credential.id + const isConfiguredByOther + = !!credential.created_by && !!userProfile?.id && credential.created_by !== userProfile.id + const isBorrowed + = !!credential.from_other_member || (isSelected && isConfiguredByOther && isPersonal) + const showSwitchAwayHint = isBorrowed const showAction = useMemo(() => { return !(disableRename && disableEdit && disableDelete && disableSetDefault) }, [disableRename, disableEdit, disableDelete, disableSetDefault]) @@ -147,9 +161,25 @@ const Item = ({ ) } { - credential.from_enterprise && ( + showSwitchAwayHint && ( + + + +
+ )} + /> + + {t('auth.onlyAtCreationHintTooltip', { ns: 'plugin' })} + + + ) + } + { + !showSwitchAwayHint && credential.from_enterprise && ( - Enterprise + {t('auth.enterprise', { ns: 'plugin' })} ) } @@ -157,7 +187,7 @@ const Item = ({ showAction && !renaming && (
{ - !credential.is_default && !disableSetDefault && !credential.not_allowed_to_use && ( + !credential.is_default && !disableSetDefault && !credential.not_allowed_to_use && !isBorrowed && (