Fix: missing authorization checks in /files/link-to-datasets (#14649)

### Related issues
Closes #14648

### What problem does this PR solve?

This PR fixes an authorization flaw in `POST /files/link-to-datasets`.

Before this change, the endpoint only checked whether the supplied
`file_ids` and `kb_ids` existed. It did not verify whether the
authenticated user was actually allowed to access those files or target
datasets. As a result, an authenticated user who knew valid IDs could
relink another user's files to arbitrary datasets.

This was especially risky because the relinking flow is state-changing:
the background worker removes existing file-document mappings and then
recreates documents under the attacker-supplied dataset IDs.

This change makes the route enforce the same permission model already
used by nearby file and document operations:

- each resolved file must pass `check_file_team_permission(...)`
- each target dataset must pass `check_kb_team_permission(...)`
- authorization is enforced before scheduling background relinking work

### 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 regression coverage in
`test/testcases/test_web_api/test_file_app/test_file2document_routes_unit.py`
- Covered:
  - unauthorized file access is rejected
  - unauthorized dataset access is rejected
- existing success path still returns immediately after scheduling
background work
- Attempted to run:
- `python -m pytest
test\\testcases\\test_web_api\\test_file_app\\test_file2document_routes_unit.py
-q`
- Local execution in this workspace is currently blocked by missing test
dependencies during bootstrap, including `ragflow_sdk`

---------

Co-authored-by: jony376 <jony376@gmail.com>
This commit is contained in:
jony376
2026-05-07 22:49:23 -07:00
committed by GitHub
parent f703169117
commit 6547751936
2 changed files with 86 additions and 18 deletions

View File

@@ -18,6 +18,7 @@ import asyncio
import logging
from pathlib import Path
from api.common.check_team_permission import check_file_team_permission, check_kb_team_permission
from api.db.services.file2document_service import File2DocumentService
from api.db.services.file_service import FileService
@@ -28,6 +29,8 @@ from common.misc_utils import get_uuid
from api.db import FileType
from api.db.services.document_service import DocumentService
logger = logging.getLogger(__name__)
def _convert_files(file_ids, kb_ids, user_id):
"""Synchronous worker: delete old docs and insert new ones for the given file/kb pairs."""
@@ -88,13 +91,29 @@ async def convert():
# Validate all files exist before starting any work
for file_id in file_ids:
if not files_set.get(file_id):
logger.warning(
"user_id=%s resource_type=file resource_id=%s action=validate_file_lookup result=not_found file_ids=%s kb_ids=%s",
current_user.id,
file_id,
file_ids,
kb_ids,
)
return get_data_error_result(message="File not found!")
# Validate all kb_ids exist before scheduling background work
kb_map = {}
for kb_id in kb_ids:
e, _ = KnowledgebaseService.get_by_id(kb_id)
e, kb = KnowledgebaseService.get_by_id(kb_id)
if not e:
logger.warning(
"user_id=%s resource_type=dataset resource_id=%s action=validate_dataset_lookup result=not_found file_ids=%s kb_ids=%s",
current_user.id,
kb_id,
file_ids,
kb_ids,
)
return get_data_error_result(message="Can't find this dataset!")
kb_map[kb_id] = kb
# Expand folders to their innermost file IDs
all_file_ids = []
@@ -106,6 +125,38 @@ async def convert():
all_file_ids.append(file_id)
user_id = current_user.id
for file_id in all_file_ids:
e, file = FileService.get_by_id(file_id)
if not e or not file:
logger.warning(
"user_id=%s resource_type=file resource_id=%s action=validate_expanded_file_lookup result=not_found file_ids=%s kb_ids=%s",
user_id,
file_id,
file_ids,
kb_ids,
)
return get_data_error_result(message="File not found!")
if not check_file_team_permission(file, user_id):
logger.warning(
"user_id=%s resource_type=file resource_id=%s action=authorize_file result=denied file_ids=%s kb_ids=%s",
user_id,
file_id,
file_ids,
kb_ids,
)
return get_data_error_result(message="No authorization.")
for kb_id, kb in kb_map.items():
if not check_kb_team_permission(kb, user_id):
logger.warning(
"user_id=%s resource_type=dataset resource_id=%s action=authorize_dataset result=denied file_ids=%s kb_ids=%s",
user_id,
kb_id,
file_ids,
kb_ids,
)
return get_data_error_result(message="No authorization.")
# Run the blocking DB work in a thread so the event loop is not blocked.
# For large folders this prevents 504 Gateway Timeout by returning as
# soon as the background task is scheduled.
@@ -114,6 +165,12 @@ async def convert():
future.add_done_callback(
lambda f: logging.error("_convert_files failed: %s", f.exception()) if f.exception() else None
)
logger.info(
"user_id=%s resource_type=file_to_dataset_link resource_id=batch action=schedule_convert result=scheduled file_ids=%s kb_ids=%s",
user_id,
all_file_ids,
kb_ids,
)
return get_json_result(data=True)
except Exception as e:
return server_error_response(e)

View File

@@ -34,17 +34,6 @@ class _DummyManager:
return decorator
class _AwaitableValue:
def __init__(self, value):
self._value = value
def __await__(self):
async def _co():
return self._value
return _co().__await__()
class _DummyFile:
def __init__(self, file_id, file_type, *, name="file.txt", location="loc", size=1):
self.id = file_id
@@ -109,6 +98,16 @@ def _load_file2document_module(monkeypatch):
services_pkg.__path__ = []
monkeypatch.setitem(sys.modules, "api.db.services", services_pkg)
common_pkg = ModuleType("api.common")
common_pkg.__path__ = []
monkeypatch.setitem(sys.modules, "api.common", common_pkg)
permission_mod = ModuleType("api.common.check_team_permission")
permission_mod.check_file_team_permission = lambda *_args, **_kwargs: True
permission_mod.check_kb_team_permission = lambda *_args, **_kwargs: True
monkeypatch.setitem(sys.modules, "api.common.check_team_permission", permission_mod)
common_pkg.check_team_permission = permission_mod
file2document_mod = ModuleType("api.db.services.file2document_service")
class _StubFile2DocumentService:
@@ -244,25 +243,37 @@ def test_convert_branch_matrix_unit(monkeypatch):
req_state = {"kb_ids": ["kb-1"], "file_ids": ["f1"]}
_set_request_json(monkeypatch, module, req_state)
# Falsy file "File not found!" (synchronous validation)
# Falsy file returns "File not found!" during synchronous validation.
monkeypatch.setattr(module.FileService, "get_by_ids", lambda _ids: [_FalsyFile("f1", module.FileType.DOC.value)])
res = _run(module.convert())
assert res["message"] == "File not found!"
# Valid file but invalid kb "Can't find this dataset!" (synchronous validation)
# KnowledgebaseService stub returns (False, None) by default
# Valid file but invalid kb returns "Can't find this dataset!" during synchronous validation.
monkeypatch.setattr(module.FileService, "get_by_ids", lambda _ids: [_DummyFile("f1", module.FileType.DOC.value)])
res = _run(module.convert())
assert res["message"] == "Can't find this dataset!"
# Valid file and kb → schedules background work, returns data=True immediately
kb = SimpleNamespace(id="kb-1", parser_id="naive", pipeline_id="p1", parser_config={})
monkeypatch.setattr(module.KnowledgebaseService, "get_by_id", lambda _kb_id: (True, kb))
# Unauthorized file access is rejected before scheduling background work.
monkeypatch.setattr(module, "check_file_team_permission", lambda *_args, **_kwargs: False)
res = _run(module.convert())
assert res["message"] == "No authorization."
# Unauthorized dataset access is rejected before scheduling background work.
monkeypatch.setattr(module, "check_file_team_permission", lambda *_args, **_kwargs: True)
monkeypatch.setattr(module, "check_kb_team_permission", lambda *_args, **_kwargs: False)
res = _run(module.convert())
assert res["message"] == "No authorization."
# Valid file and kb schedule background work and return data=True immediately.
monkeypatch.setattr(module, "check_kb_team_permission", lambda *_args, **_kwargs: True)
res = _run(module.convert())
assert res["code"] == 0
assert res["data"] is True
# Folder expansion schedules background work, returns data=True immediately
# Folder expansion schedules background work and returns data=True immediately.
req_state["file_ids"] = ["folder-1"]
monkeypatch.setattr(module.FileService, "get_by_ids", lambda _ids: [_DummyFile("folder-1", module.FileType.FOLDER.value, name="folder")])
monkeypatch.setattr(module.FileService, "get_all_innermost_file_ids", lambda _file_id, _acc: ["inner-1"])
@@ -270,7 +281,7 @@ def test_convert_branch_matrix_unit(monkeypatch):
assert res["code"] == 0
assert res["data"] is True
# Exception in file lookup 500
# Exception in file lookup returns 500.
req_state["file_ids"] = ["f1"]
monkeypatch.setattr(
module.FileService,