From 3b6eeabb09613af6bf27d272670bff1488643344 Mon Sep 17 00:00:00 2001 From: jony376 Date: Fri, 8 May 2026 22:30:14 -0700 Subject: [PATCH] Fix: private dataset authorization bypass in shared dataset access checks (#14645) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### Related issues Closes #14644 ### What problem does this PR solve? This PR fixes an authorization bug where datasets marked with `permission = me` could still be accessed by other members of the same tenant through APIs that relied on `KnowledgebaseService.accessible()` or `DocumentService.accessible()`. Before this change, those shared access helpers only checked tenant membership and did not enforce the dataset's permission mode. As a result, a non-owner who knew a private `dataset_id` could still reach downstream document and chunk operations even though the dataset was intended to be owner-only. This change updates the central access checks so that: - dataset owners always retain access - joined tenant members only get access when the dataset permission is `TEAM` - private datasets with `permission = me` remain inaccessible to non-owners - document-level access follows the same dataset permission rules The PR also adds regression coverage for private-vs-team dataset access behavior. ### Type of change - [x] Bug Fix (non-breaking change which fixes an issue) - [ ] New Feature (non-breaking change which adds functionality) - [ ] Documentation Update - [ ] Refactoring - [ ] Performance Improvement - [ ] Other (please describe): ### Testing - Added `test/unit_test/api/db/services/test_dataset_access_permissions.py` - Attempted to run: `python -m pytest test\\unit_test\\api\\db\\services\\test_dataset_access_permissions.py -q` - Local execution in this workspace is currently blocked during test collection because the environment is missing the `strenum` dependency --------- Signed-off-by: Jin Hai Co-authored-by: jony376 Co-authored-by: Wang Qi Co-authored-by: d 🔹 Co-authored-by: Jin Hai Co-authored-by: Magicbook1108 Co-authored-by: chanx <1243304602@qq.com> Co-authored-by: sxxtony <166789813+sxxtony@users.noreply.github.com> Co-authored-by: sxxtony Co-authored-by: Baki Burak Öğün <63836730+bakiburakogun@users.noreply.github.com> Co-authored-by: bakiburakogun Co-authored-by: Panda Dev <56657208+pandadev66@users.noreply.github.com> Co-authored-by: Haruko386 Co-authored-by: D2758695161 <13510221939@163.com> Co-authored-by: Hunter Co-authored-by: Lynn Co-authored-by: buua436 Co-authored-by: web-dev0521 Co-authored-by: Tim Wang <38489718+wanghualoong@users.noreply.github.com> Co-authored-by: wanghualoong Co-authored-by: Claude Opus 4.6 Co-authored-by: qinling0210 <88864212+qinling0210@users.noreply.github.com> Co-authored-by: dale053 --- api/db/services/document_service.py | 13 +- api/db/services/knowledgebase_service.py | 39 +++--- .../test_dataset_access_permissions.py | 119 ++++++++++++++++++ 3 files changed, 146 insertions(+), 25 deletions(-) create mode 100644 test/unit_test/api/db/services/test_dataset_access_permissions.py diff --git a/api/db/services/document_service.py b/api/db/services/document_service.py index 5d6289e573..7992cdb610 100644 --- a/api/db/services/document_service.py +++ b/api/db/services/document_service.py @@ -678,17 +678,10 @@ class DocumentService(CommonService): @classmethod @DB.connection_context() def accessible(cls, doc_id, user_id): - docs = ( - cls.model.select(cls.model.id) - .join(Knowledgebase, on=(Knowledgebase.id == cls.model.kb_id)) - .join(UserTenant, on=(UserTenant.tenant_id == Knowledgebase.tenant_id)) - .where(cls.model.id == doc_id, UserTenant.user_id == user_id) - .paginate(0, 1) - ) - docs = docs.dicts() - if not docs: + e, doc = cls.get_by_id(doc_id) + if not e: return False - return True + return KnowledgebaseService.accessible(doc.kb_id, user_id) @classmethod @DB.connection_context() diff --git a/api/db/services/knowledgebase_service.py b/api/db/services/knowledgebase_service.py index c66d66a682..a164287fa4 100644 --- a/api/db/services/knowledgebase_service.py +++ b/api/db/services/knowledgebase_service.py @@ -18,7 +18,7 @@ from datetime import datetime from peewee import fn, JOIN from api.db import TenantPermission -from api.db.db_models import DB, Document, Knowledgebase, User, UserTenant, UserCanvas +from api.db.db_models import DB, Document, Knowledgebase, User, UserCanvas from api.db.services.common_service import CommonService from common.time_utils import current_timestamp, datetime_format from api.db.services import duplicate_name @@ -485,13 +485,21 @@ class KnowledgebaseService(CommonService): # user_id: User ID # Returns: # Boolean indicating accessibility - docs = cls.model.select( - cls.model.id).join(UserTenant, on=(UserTenant.tenant_id == Knowledgebase.tenant_id) - ).where(cls.model.id == kb_id, UserTenant.user_id == user_id).paginate(0, 1) - docs = docs.dicts() - if not docs: + e, kb = cls.get_by_id(kb_id) + if not e: return False - return True + + if kb.status != StatusEnum.VALID.value: + return False + + if kb.tenant_id == user_id: + return True + + if kb.permission != TenantPermission.TEAM.value: + return False + + joined_tenants = TenantService.get_joined_tenants_by_user_id(user_id) + return any(tenant["tenant_id"] == kb.tenant_id for tenant in joined_tenants) @classmethod @DB.connection_context() @@ -502,10 +510,10 @@ class KnowledgebaseService(CommonService): # user_id: User ID # Returns: # List containing dataset information - kbs = cls.model.select().join(UserTenant, on=(UserTenant.tenant_id == Knowledgebase.tenant_id) - ).where(cls.model.id == kb_id, UserTenant.user_id == user_id).paginate(0, 1) - kbs = kbs.dicts() - return list(kbs) + e, kb = cls.get_by_id(kb_id) + if not e or not cls.accessible(kb_id, user_id): + return [] + return [kb.to_dict()] @classmethod @DB.connection_context() @@ -516,10 +524,11 @@ class KnowledgebaseService(CommonService): # user_id: User ID # Returns: # List containing dataset information - kbs = cls.model.select().join(UserTenant, on=(UserTenant.tenant_id == Knowledgebase.tenant_id) - ).where(cls.model.name == kb_name, UserTenant.user_id == user_id).paginate(0, 1) - kbs = kbs.dicts() - return list(kbs) + kbs = cls.query(name=kb_name, status=StatusEnum.VALID.value) + for kb in kbs: + if cls.accessible(kb.id, user_id): + return [kb.to_dict()] + return [] @classmethod @DB.connection_context() diff --git a/test/unit_test/api/db/services/test_dataset_access_permissions.py b/test/unit_test/api/db/services/test_dataset_access_permissions.py new file mode 100644 index 0000000000..e3db6d0f2a --- /dev/null +++ b/test/unit_test/api/db/services/test_dataset_access_permissions.py @@ -0,0 +1,119 @@ +# +# Copyright 2026 The InfiniFlow Authors. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +import sys +import types +import warnings +from types import SimpleNamespace + +# xgboost imports pkg_resources and emits a deprecation warning that is promoted +# to error in our pytest configuration; ignore it for this unit test module. +warnings.filterwarnings( + "ignore", + message="pkg_resources is deprecated as an API.*", + category=UserWarning, +) + + +def _install_cv2_stub_if_unavailable(): + try: + import cv2 # noqa: F401 + return + except Exception: + pass + + stub = types.ModuleType("cv2") + + stub.INTER_LINEAR = 1 + stub.INTER_CUBIC = 2 + stub.BORDER_CONSTANT = 0 + stub.BORDER_REPLICATE = 1 + stub.COLOR_BGR2RGB = 0 + stub.COLOR_BGR2GRAY = 1 + stub.COLOR_GRAY2BGR = 2 + stub.IMREAD_IGNORE_ORIENTATION = 128 + stub.IMREAD_COLOR = 1 + stub.RETR_LIST = 1 + stub.CHAIN_APPROX_SIMPLE = 2 + + def _missing(*_args, **_kwargs): + raise RuntimeError("cv2 runtime call is unavailable in this test environment") + + def _module_getattr(name): + if name.isupper(): + return 0 + return _missing + + stub.__getattr__ = _module_getattr + sys.modules["cv2"] = stub + + +_install_cv2_stub_if_unavailable() + +from api.db import TenantPermission +from api.db.services.document_service import DocumentService +from api.db.services.knowledgebase_service import KnowledgebaseService +from common.constants import StatusEnum + + +def _unwrapped_kb_accessible(): + return KnowledgebaseService.accessible.__func__.__wrapped__ + + +def _unwrapped_doc_accessible(): + return DocumentService.accessible.__func__.__wrapped__ + + +def test_private_dataset_is_not_accessible_to_other_tenant_member(monkeypatch): + kb = SimpleNamespace( + id="kb-private", + tenant_id="owner-1", + permission=TenantPermission.ME.value, + status=StatusEnum.VALID.value, + ) + + monkeypatch.setattr(KnowledgebaseService, "get_by_id", classmethod(lambda cls, kb_id: (True, kb))) + monkeypatch.setattr( + "api.db.services.knowledgebase_service.TenantService.get_joined_tenants_by_user_id", + lambda _user_id: [{"tenant_id": "owner-1"}], + ) + + assert _unwrapped_kb_accessible()(KnowledgebaseService, "kb-private", "member-2") is False + + +def test_team_dataset_is_accessible_to_joined_tenant_member(monkeypatch): + kb = SimpleNamespace( + id="kb-team", + tenant_id="owner-1", + permission=TenantPermission.TEAM.value, + status=StatusEnum.VALID.value, + ) + + monkeypatch.setattr(KnowledgebaseService, "get_by_id", classmethod(lambda cls, kb_id: (True, kb))) + monkeypatch.setattr( + "api.db.services.knowledgebase_service.TenantService.get_joined_tenants_by_user_id", + lambda _user_id: [{"tenant_id": "owner-1"}], + ) + + assert _unwrapped_kb_accessible()(KnowledgebaseService, "kb-team", "member-2") is True + + +def test_document_access_respects_dataset_permission(monkeypatch): + doc = SimpleNamespace(id="doc-1", kb_id="kb-private") + + monkeypatch.setattr(DocumentService, "get_by_id", classmethod(lambda cls, doc_id: (True, doc))) + monkeypatch.setattr(KnowledgebaseService, "accessible", classmethod(lambda cls, kb_id, user_id: False)) + + assert _unwrapped_doc_accessible()(DocumentService, "doc-1", "member-2") is False