fix: report a locked folder as FolderLockedException
Fetching a locked folder before its password is sent returns 200 with only its listing fields, without metadata, sub_folders and path, so FoldersApi.retrieve failed with a pydantic ValidationError. The DOI route fetches the folder before it can see the lock, so a password-protected DOI folder could not be opened even with its password. - Add FolderLockedException, a subclass of UnauthorizedException, and raise it from retrieve on that short record and from the lock checks. - On a DOI folder, send the password and fetch it again when the first fetch finds it locked.
This commit is contained in:
@@ -12,6 +12,7 @@ All notable changes to this project will be documented in this file.
|
|||||||
|
|
||||||
### Fixed
|
### 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 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
|
## [1.3.20] - 2026-11-04
|
||||||
|
|
||||||
|
|||||||
@@ -6,7 +6,7 @@ from pydantic.dataclasses import dataclass
|
|||||||
|
|
||||||
from mdrsclient.api.base import BaseApi
|
from mdrsclient.api.base import BaseApi
|
||||||
from mdrsclient.api.utils import retry_on_expired_token, token_check
|
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
|
from mdrsclient.models import Folder, FolderSimple
|
||||||
|
|
||||||
|
|
||||||
@@ -38,7 +38,12 @@ class FoldersApi(BaseApi):
|
|||||||
token_check(self.connection)
|
token_check(self.connection)
|
||||||
response = self.connection.get(url)
|
response = self.connection.get(url)
|
||||||
self._raise_response_error(response)
|
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
|
return ret
|
||||||
|
|
||||||
@retry_on_expired_token
|
@retry_on_expired_token
|
||||||
|
|||||||
@@ -28,6 +28,12 @@ class UnauthorizedException(MDRSException):
|
|||||||
pass
|
pass
|
||||||
|
|
||||||
|
|
||||||
|
class FolderLockedException(UnauthorizedException):
|
||||||
|
"""Thrown when a folder cannot be opened without its password"""
|
||||||
|
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
class TokenExpiredException(UnauthorizedException):
|
class TokenExpiredException(UnauthorizedException):
|
||||||
"""Thrown when the access token was still valid when sent but had expired on arrival"""
|
"""Thrown when the access token was still valid when sent but had expired on arrival"""
|
||||||
|
|
||||||
|
|||||||
+13
-5
@@ -8,9 +8,9 @@ from mdrsclient.cache import CacheInterface
|
|||||||
from mdrsclient.config import ConfigFile, ConfigInterface
|
from mdrsclient.config import ConfigFile, ConfigInterface
|
||||||
from mdrsclient.connection import MDRSConnection
|
from mdrsclient.connection import MDRSConnection
|
||||||
from mdrsclient.exceptions import (
|
from mdrsclient.exceptions import (
|
||||||
|
FolderLockedException,
|
||||||
IllegalArgumentException,
|
IllegalArgumentException,
|
||||||
MissingConfigurationException,
|
MissingConfigurationException,
|
||||||
UnauthorizedException,
|
|
||||||
UnexpectedException,
|
UnexpectedException,
|
||||||
)
|
)
|
||||||
from mdrsclient.models import File, Folder, Laboratory, Token, User
|
from mdrsclient.models import File, Folder, Laboratory, Token, User
|
||||||
@@ -103,7 +103,7 @@ class MdrsService:
|
|||||||
raise UnexpectedException(f"Folder `{path}` not found.")
|
raise UnexpectedException(f"Folder `{path}` not found.")
|
||||||
if folders[0].lock:
|
if folders[0].lock:
|
||||||
if password is None:
|
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)
|
folder_api.auth(folders[0].id, password)
|
||||||
return folder_api.retrieve(folders[0].id)
|
return folder_api.retrieve(folders[0].id)
|
||||||
|
|
||||||
@@ -207,12 +207,20 @@ class MdrsService:
|
|||||||
)
|
)
|
||||||
|
|
||||||
folder_api = FoldersApi(self.connection)
|
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:
|
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)
|
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)
|
lab_api = LaboratoriesApi(self.connection)
|
||||||
labs = lab_api.list()
|
labs = lab_api.list()
|
||||||
|
|||||||
+55
-6
@@ -1,6 +1,8 @@
|
|||||||
import unittest
|
import unittest
|
||||||
from unittest.mock import MagicMock, patch
|
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.models import Doi, Folder, Laboratories, Laboratory
|
||||||
from mdrsclient.services import MdrsService
|
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):
|
class TestFindFolderByDoi(unittest.TestCase):
|
||||||
"""The DOI response carries the folder id as a field, not as a nested object."""
|
"""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 = Laboratories()
|
||||||
laboratories.append(LABORATORY)
|
laboratories.append(LABORATORY)
|
||||||
|
|
||||||
@@ -41,14 +76,28 @@ class TestFindFolderByDoi(unittest.TestCase):
|
|||||||
patch("mdrsclient.services.LaboratoriesApi") as laboratories_api,
|
patch("mdrsclient.services.LaboratoriesApi") as laboratories_api,
|
||||||
):
|
):
|
||||||
doi_api.return_value.retrieve.return_value = Doi(id="20260429-001", doi=DOI, folder_id="f1")
|
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
|
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")
|
def test_a_locked_doi_folder_without_a_password(self):
|
||||||
self.assertEqual(folder.id, "f1")
|
with self.assertRaisesRegex(FolderLockedException, f"Folder for DOI `{DOI}` is locked."):
|
||||||
self.assertEqual(laboratory.name, "mylab")
|
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__":
|
if __name__ == "__main__":
|
||||||
|
|||||||
Reference in New Issue
Block a user