mirror of
https://github.com/langgenius/dify.git
synced 2026-05-10 14:14:17 +08:00
fix(tools): scope builtin tool default-credential clear to tenant
When two workspace members each set their own credential as the default for the same provider, both rows ended up with is_default=True for the same (tenant_id, provider). The clear UPDATE was filtered by user_id, so it only reset the caller's own defaults and left other members' defaults intact. The consumer (`get_builtin_provider`) is tenant-scoped and picks an arbitrary winner via the created_at tiebreak when multiple defaults exist, producing user-visible inconsistencies. Drop user_id from the clear filter so default selection becomes properly tenant-scoped, matching `DatasourceProviderService.set_default_datasource_provider`. Adds a regression test that asserts the compiled UPDATE WHERE clause contains tenant_id/provider but not user_id.
This commit is contained in:
parent
cd66559ebf
commit
7b22d813ed
@ -422,12 +422,11 @@ class BuiltinToolManageService:
|
||||
if target_provider is None:
|
||||
raise ValueError("provider not found")
|
||||
|
||||
# clear default provider
|
||||
# clear default provider (tenant-scoped: only one default per provider per workspace)
|
||||
session.execute(
|
||||
update(BuiltinToolProvider)
|
||||
.where(
|
||||
BuiltinToolProvider.tenant_id == tenant_id,
|
||||
BuiltinToolProvider.user_id == user_id,
|
||||
BuiltinToolProvider.provider == provider,
|
||||
BuiltinToolProvider.is_default.is_(True),
|
||||
)
|
||||
|
||||
@ -194,6 +194,24 @@ class TestSetDefaultProvider:
|
||||
assert result == {"result": "success"}
|
||||
assert target.is_default is True
|
||||
|
||||
@patch(f"{MODULE}.sessionmaker")
|
||||
@patch(f"{MODULE}.db")
|
||||
def test_clear_default_is_tenant_scoped_not_user_scoped(self, mock_db, mock_sm_cls):
|
||||
# Regression: clearing prior defaults must NOT filter by user_id, otherwise
|
||||
# two workspace members can each leave their own credential as default at
|
||||
# the same time (the default flag is tenant-scoped, not per-user).
|
||||
session = _mock_sessionmaker(mock_sm_cls)
|
||||
session.scalar.return_value = MagicMock()
|
||||
|
||||
BuiltinToolManageService.set_default_provider("tenant-1", "user-A", "google", "cred-id")
|
||||
|
||||
session.execute.assert_called_once()
|
||||
update_stmt = session.execute.call_args.args[0]
|
||||
compiled = str(update_stmt.compile(compile_kwargs={"literal_binds": True}))
|
||||
assert "user_id" not in compiled
|
||||
assert "tenant_id" in compiled
|
||||
assert "provider" in compiled
|
||||
|
||||
|
||||
class TestUpdateBuiltinToolProvider:
|
||||
@patch(f"{MODULE}.sessionmaker")
|
||||
|
||||
Loading…
Reference in New Issue
Block a user