Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion dms/controllers/portal.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,11 @@ def _get_files(self, access_token, dms_directory_id, search, search_in, sort_br)

# items
file_model = request.env["dms.file"]
is_access_token_valid = file_model.check_access_token(access_token)
# The token belongs to a directory, so it must be validated against the
# directory being browsed (or one of its ancestors), never against an
# empty dms.file recordset. Same pattern as _get_directories below.
directory_to_check = request.env["dms.directory"].browse(dms_directory_id)
is_access_token_valid = directory_to_check.check_access_token(access_token)
file_model = file_model.sudo() if is_access_token_valid else file_model
dms_file_items = file_model.search(file_domain, order=sort_br)
request.session["my_dms_file_history"] = dms_file_items.ids
Expand Down
7 changes: 6 additions & 1 deletion dms/models/directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -254,10 +254,15 @@ def check_access_token(self, access_token=False):
return True
# sudo because the user might not usually have access to the record but
# now the token is valid.
# `seen` bounds the walk: _check_directory_recursion rejects
# cycles created through the ORM, but this path is reachable
# anonymously and must not hang on corrupted data.
directory_item = self.sudo()
while directory_item.parent_id:
seen = set()
while directory_item.parent_id and directory_item.id not in seen:
if directory_item.id == item.id:
return True
seen.add(directory_item.id)
directory_item = directory_item.parent_id
# Fix last level
if directory_item.id == item.id:
Expand Down
19 changes: 13 additions & 6 deletions dms/models/dms_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,15 +174,22 @@ def check_access_token(self, access_token=False):
)
if items:
item = items[0]
if self.directory_id.id == item.id:
return True
directory_item = self.directory_id
while directory_item.parent_id:
if directory_item.id == self.directory_id.id:
# The token is known to belong to some directory, but it is not yet
# valid for this file: it only is when that directory is the file's
# own directory or one of its ancestors. sudo() so the walk can
# traverse ancestors the caller is not allowed to read.
# `seen` bounds the walk: _check_directory_recursion rejects cycles
# created through the ORM, but this path is reachable anonymously
# and must not hang on corrupted data.
directory_item = self.sudo().directory_id
seen = set()
while directory_item.parent_id and directory_item.id not in seen:
if directory_item.id == item.id:
return True
seen.add(directory_item.id)
directory_item = directory_item.parent_id
# Fix last level
if directory_item.id == self.directory_id.id:
if directory_item.id == item.id:
return True
return False

Expand Down
1 change: 1 addition & 0 deletions dms/tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@
from . import test_file
from . import test_benchmark
from . import test_portal
from . import test_access_token
92 changes: 92 additions & 0 deletions dms/tests/test_access_token.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
# Copyright 2026 Millow AB
# License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl).
"""Regression tests for share-token scoping.

``dms.file.check_access_token`` used to compare the directory walker against
itself instead of against the directory that owns the token, which made the
whole branch collapse to ``return True``. Any valid directory token therefore
granted read access to *every* file in the database, including files in
unrelated trees and in directories with broken group inheritance.
"""

import uuid

from .common import StorageFileBaseCase


class TestDmsAccessToken(StorageFileBaseCase):
@classmethod
def setUpClass(cls):
super().setUpClass()
# Two unrelated trees under the same storage.
# shared_root / shared_child <- the token lives on shared_root
# other_root <- must stay unreachable
cls.shared_root = cls.create_directory(storage=cls.storage)
cls.shared_child = cls.create_directory(directory=cls.shared_root)
cls.other_root = cls.create_directory(storage=cls.storage)

cls.file_in_shared_root = cls.create_file(directory=cls.shared_root)
cls.file_in_shared_child = cls.create_file(directory=cls.shared_child)
cls.file_in_other_root = cls.create_file(directory=cls.other_root)

cls.token = uuid.uuid4().hex
cls.shared_root.access_token = cls.token

# ------------------------------------------------------------------
# The defect
# ------------------------------------------------------------------
def test_token_does_not_grant_access_to_unrelated_tree(self):
"""A token on shared_root must NOT unlock a file under other_root."""
self.assertFalse(
self.file_in_other_root.check_access_token(self.token),
"Directory token leaked into an unrelated directory tree",
)

def test_token_does_not_grant_access_to_root_level_file(self):
"""The `# Fix last level` branch self-compared too.

A file whose directory has no parent skipped the loop entirely and
fell through to a second always-true comparison, so root-level files
leaked as well.
"""
self.assertFalse(
self.file.check_access_token(self.token),
"Directory token leaked into a root-level file of another tree",
)

def test_unrelated_token_value_is_rejected(self):
self.assertFalse(
self.file_in_shared_child.check_access_token(uuid.uuid4().hex),
"An unknown token value was accepted",
)

def test_no_token_is_rejected(self):
self.assertFalse(self.file_in_shared_child.check_access_token(False))

# ------------------------------------------------------------------
# Legitimate behaviour that must keep working
# ------------------------------------------------------------------
def test_token_grants_access_to_file_in_the_shared_directory(self):
self.assertTrue(
self.file_in_shared_root.check_access_token(self.token),
"Token did not unlock a file in its own directory",
)

def test_token_grants_access_to_file_in_a_descendant_directory(self):
self.assertTrue(
self.file_in_shared_child.check_access_token(self.token),
"Token did not unlock a file in a descendant directory",
)

def test_file_own_token_still_works(self):
own_token = uuid.uuid4().hex
self.file_in_other_root.access_token = own_token
self.assertTrue(
self.file_in_other_root.check_access_token(own_token),
"A file's own access token stopped working",
)

def test_directory_token_grants_access_to_descendant_directory(self):
"""The directory-side implementation was already correct; pin it."""
self.assertTrue(self.shared_child.check_access_token(self.token))
self.assertFalse(self.other_root.check_access_token(self.token))
Loading