diff --git a/CHANGELOG.md b/CHANGELOG.md index 7dc7163..f1b8a92 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ All notable changes to this project will be documented in this file. ### Fixed - Reported an access level that has no meaning as `Unknown (0x…)` instead of `Storage`. Storage is the absence of every bit, so it matched any level. A level made of known bits is no longer labelled by the first table entry it contained either: `0x10104` was `CBS or PW Open`. This changes the `access_level` value in `ls` output, including JSON. +- Reported a locked folder that was fetched without its password as `FolderLockedException`, a subclass of `UnauthorizedException`, instead of a pydantic `ValidationError`. The server answers with a short record that has no sub-folders or path. A password-protected DOI folder fails this way before its password is sent, so it can now be opened again. ## [1.3.20] - 2026-11-04 diff --git a/mdrsclient/api/folders.py b/mdrsclient/api/folders.py index 6fe903b..9b41b08 100644 --- a/mdrsclient/api/folders.py +++ b/mdrsclient/api/folders.py @@ -6,7 +6,7 @@ from pydantic.dataclasses import dataclass from mdrsclient.api.base import BaseApi from mdrsclient.api.utils import retry_on_expired_token, token_check -from mdrsclient.exceptions import UnauthorizedException +from mdrsclient.exceptions import FolderLockedException, UnauthorizedException from mdrsclient.models import Folder, FolderSimple @@ -38,7 +38,12 @@ class FoldersApi(BaseApi): token_check(self.connection) response = self.connection.get(url) self._raise_response_error(response) - ret = TypeAdapter(Folder).validate_python(response.json()) + data = response.json() + # A locked folder not yet opened with its password comes back with 200 but only + # its listing fields, without `metadata`, `sub_folders` and `path`. + if data.get("lock") and "sub_folders" not in data: + raise FolderLockedException(f"Folder `{data.get('name', id)}` is locked.") + ret = TypeAdapter(Folder).validate_python(data) return ret @retry_on_expired_token diff --git a/mdrsclient/exceptions.py b/mdrsclient/exceptions.py index de37d24..4cd1e27 100644 --- a/mdrsclient/exceptions.py +++ b/mdrsclient/exceptions.py @@ -28,6 +28,12 @@ class UnauthorizedException(MDRSException): pass +class FolderLockedException(UnauthorizedException): + """Thrown when a folder cannot be opened without its password""" + + pass + + class TokenExpiredException(UnauthorizedException): """Thrown when the access token was still valid when sent but had expired on arrival""" diff --git a/mdrsclient/services.py b/mdrsclient/services.py index ff0b436..ed0c321 100644 --- a/mdrsclient/services.py +++ b/mdrsclient/services.py @@ -8,9 +8,9 @@ from mdrsclient.cache import CacheInterface from mdrsclient.config import ConfigFile, ConfigInterface from mdrsclient.connection import MDRSConnection from mdrsclient.exceptions import ( + FolderLockedException, IllegalArgumentException, MissingConfigurationException, - UnauthorizedException, UnexpectedException, ) from mdrsclient.models import File, Folder, Laboratory, Token, User @@ -103,7 +103,7 @@ class MdrsService: raise UnexpectedException(f"Folder `{path}` not found.") if folders[0].lock: if password is None: - raise UnauthorizedException(f"Folder `{path}` is locked.") + raise FolderLockedException(f"Folder `{path}` is locked.") folder_api.auth(folders[0].id, password) return folder_api.retrieve(folders[0].id) @@ -207,12 +207,20 @@ class MdrsService: ) folder_api = FoldersApi(self.connection) - folder = folder_api.retrieve(doi_resp.folder_id) + # The folder is found by its id, so whether it is locked is known only from what + # retrieve returns: a full record, or the short one of a locked folder. + folder: Folder | None + try: + folder = folder_api.retrieve(doi_resp.folder_id) + except FolderLockedException: + folder = None - if folder.lock: + if folder is None or folder.lock: if password is None: - raise UnauthorizedException(f"Folder for DOI `{doi_clean}` is locked.") + raise FolderLockedException(f"Folder for DOI `{doi_clean}` is locked.") folder_api.auth(doi_resp.folder_id, password) + if folder is None: + folder = folder_api.retrieve(doi_resp.folder_id) lab_api = LaboratoriesApi(self.connection) labs = lab_api.list() diff --git a/tests/test_services.py b/tests/test_services.py index cc9eac2..e6d3e25 100644 --- a/tests/test_services.py +++ b/tests/test_services.py @@ -1,6 +1,8 @@ import unittest from unittest.mock import MagicMock, patch +from mdrsclient.api import FoldersApi +from mdrsclient.exceptions import FolderLockedException from mdrsclient.models import Doi, Folder, Laboratories, Laboratory from mdrsclient.services import MdrsService @@ -28,10 +30,43 @@ def make_locked_folder() -> Folder: ) +# What the server returns for a locked folder not yet opened with its password. +SHORT_RECORD = { + "id": "f1", + "pid": None, + "name": "root", + "access_level": 4, + "lock": True, + "size": 0, + "laboratory_id": 1, + "description": "", + "created_at": TIMESTAMP, + "updated_at": TIMESTAMP, + "restrict_opened_at": None, + "application": {"id": 0, "status": 0}, +} + + +class TestRetrieveLockedFolder(unittest.TestCase): + def retrieve(self, data: dict) -> Folder: + connection = MagicMock() + connection.get.return_value.json.return_value = data + with patch.object(FoldersApi, "_raise_response_error"): + return FoldersApi(connection).retrieve("f1") + + def test_short_record_means_locked(self): + with self.assertRaisesRegex(FolderLockedException, "Folder `root` is locked."): + self.retrieve(SHORT_RECORD) + + def test_full_record_of_a_locked_folder_is_read(self): + full = dict(SHORT_RECORD, metadata=[], sub_folders=[], path="/root/") + self.assertEqual(self.retrieve(full).path, "/root/") + + class TestFindFolderByDoi(unittest.TestCase): """The DOI response carries the folder id as a field, not as a nested object.""" - def test_a_locked_doi_folder_is_unlocked_with_its_folder_id(self): + def find(self, retrieved: list, password: str | None) -> tuple[MagicMock, tuple]: laboratories = Laboratories() laboratories.append(LABORATORY) @@ -41,14 +76,28 @@ class TestFindFolderByDoi(unittest.TestCase): patch("mdrsclient.services.LaboratoriesApi") as laboratories_api, ): doi_api.return_value.retrieve.return_value = Doi(id="20260429-001", doi=DOI, folder_id="f1") - folders_api.return_value.retrieve.return_value = make_locked_folder() + folders_api.return_value.retrieve.side_effect = retrieved laboratories_api.return_value.list.return_value = laboratories + return folders_api.return_value, MdrsService(MagicMock()).find_folder_by_doi(DOI, password) - folder, laboratory = MdrsService(MagicMock()).find_folder_by_doi(DOI, "secret") + def test_a_locked_doi_folder_is_unlocked_with_its_folder_id(self): + api, (folder, laboratory) = self.find([FolderLockedException("locked"), make_locked_folder()], "secret") + api.auth.assert_called_once_with("f1", "secret") + self.assertEqual(api.retrieve.call_count, 2) + self.assertEqual(folder.id, "f1") + self.assertEqual(laboratory.name, "mylab") - folders_api.return_value.auth.assert_called_once_with("f1", "secret") - self.assertEqual(folder.id, "f1") - self.assertEqual(laboratory.name, "mylab") + def test_a_locked_doi_folder_without_a_password(self): + with self.assertRaisesRegex(FolderLockedException, f"Folder for DOI `{DOI}` is locked."): + self.find([FolderLockedException("locked")], None) + + def test_a_full_record_still_asks_for_the_password(self): + # A locked folder whose record is readable anyway, e.g. its owner's. + api, (folder, _) = self.find([make_locked_folder()], "secret") + api.auth.assert_called_once_with("f1", "secret") + self.assertEqual(api.retrieve.call_count, 1) + with self.assertRaises(FolderLockedException): + self.find([make_locked_folder()], None) if __name__ == "__main__":