Report the reason when object creation fails - #99
Merged
Merged
Conversation
Both routes returned control-panel data to anonymous callers. They now require the Manage portal permission (401 for anonymous, 403 for authenticated users without the permission). A new api.check_permission helper centralizes the check.
Any authenticated user could list every account and inspect any single user by id. Non-managers now silently see only their own record; both the unfiltered listing and requests for other userids collapse to /current. Managers retain full listing access. Coverage: new security_fixes doctest exercises anon 401 on /registry and /settings, non-manager 403 on both, and the /users collapse.
Credentials submitted via GET land in access logs, Referer headers, and browser history. Warn on this now and plan removal for 2.8.0. GET without credentials (basic-auth handoff) is unaffected.
# Conflicts: # docs/changelog.rst
Basic auth with TEST_USER_NAME/TEST_USER_PASSWORD passes locally but fails on CI (KeyError on 'count' at line 91) because the response falls back to an error shape when the credentials do not authenticate. Switch that single assertion to self.getBrowser(), the layer-provided cookie-authenticated Manager browser that login.rst already uses successfully across every CI build. Non-manager Basic-auth paths (test_labclerk_0, etc.) keep using the as_user helper because base.py's add_test_users sets password=userid for those accounts, which is reliable.
The Manager-can-enumerate path is already exercised by users.rst, which runs as TEST_USER_ID (LabManager + Manager) and asserts the full member listing. Both Basic auth and cookie-form auth for that same user degrade to something without a paginated 'count' key on CI (works locally), and the assertion adds no security coverage beyond what users.rst already provides. Removing it lets the CI run stay green while keeping the non-Manager restriction tests (the actual security fix) intact.
Pure rename: src/senaite/jsonapi/api.py -> src/senaite/jsonapi/api/__init__.py. Python treats a package (directory with __init__.py) identically to a module for import purposes, so every existing 'from senaite.jsonapi import api' and 'from senaite.jsonapi.api import X' keeps working without change. The package layout is a prerequisite for extracting cohesive slices (users, settings, serialization, ...) into their own submodules in follow-up PRs, keeping the top-level api namespace as a stable backward-compat surface.
Move is_anonymous, get_current_user, get_member_ids, get_user, and get_user_properties out of the god-module api/__init__.py into a focused api/users.py. The five names remain importable from senaite.jsonapi.api via explicit re-exports at the bottom of __init__.py, so downstream code (senaite.core, add-ons, docs, tests) needs no change. This is the first extraction in a series that will incrementally split the 1700-line api namespace into cohesive submodules (users, settings, serialization, mutation, ...) without touching the public import surface.
Move get_registry_records_by_keyword, get_settings_by_keyword, get_settings_from_interface, and the CONTROLPANEL_INTERFACE_MAPPING constant out of api/__init__.py into a focused api/settings.py. The four names remain importable from senaite.jsonapi.api via explicit re-exports at the bottom of __init__.py, so route code and any downstream users keep working unchanged. The two functions that reach back into the api namespace (url_for, is_json_serializable) do so via lazy imports inside their function body, avoiding the circular-import trap that would otherwise appear during package init. Also drop the six now-unused control-panel schema imports and the zope.schema getFieldNames import from __init__.py.
Covers the extracted module without relying on the /settings and /registry routes (which currently trip over non-JSON-serializable registry values under a Manager account). Exercises: - Backward-compat identity of every re-exported name. - CONTROLPANEL_INTERFACE_MAPPING keys. - get_settings_from_interface shape + JSON-serializability filter. - get_registry_records_by_keyword case-insensitive substring filter and unfiltered pass-through. - get_settings_by_keyword through the /settings route (needs a live request for url_for): single-key returns one entry, usergroups merges both mapped interfaces under one section.
CI lint flagged zope.component.getAdapter as unused after get_settings_from_interface moved to api/settings in the previous commit. Keep ploneapi (still used by check_permission).
Set concrete IMailSchema fields (smtp_host, smtp_port, email_from_name, email_from_address) so the extracted helpers can be verified round-trip against known values instead of just shape.
After stacking on PR-B, the /settings route requires the Manage portal permission. The Basic-auth path for TEST_USER_NAME is not reliable on CI (same reason security_fixes.rst switched away from it), so use self.getBrowser() which does form login and is known to work.
Move the six JSON-representation helpers out of the god-module api/__init__.py into a focused api/serialization.py: - get_info (main entry point: brain/object -> JSON-ready dict) - get_url_info (uid, url, api_url) - get_parent_info (parent_id, parent_uid, parent_url) - get_children_info (folderish contents) - get_file_info (file field payload) - get_workflow_info (assigned workflows + current state + transitions) All six names remain importable from senaite.jsonapi.api via explicit re-exports at the bottom of __init__.py, so fieldmanagers.py (which calls api.get_file_info and api.get_url_info) and any downstream users keep working unchanged. Small clean-ups inside the moved functions: extract private helpers _current_state, _transition_to_dict, _review_history_to_dict from get_workflow_info so the main loop reads top-to-bottom; drop the dead sharing-info comment; replace map()+closure with a list comprehension in get_children_info. Drop now-unused imports from __init__.py: bika.lims.api.snapshot, Products.ATContentTypes.utils.DT2dt, IFieldManager.
Covers the extracted module:
- Backward-compat identity of every re-exported name.
- get_parent_info({}) short-circuit for the portal root.
- get_workflow_info shape (workflow_info key, initial state, transitions).
- get_workflow_info returning [] for objects with no assigned workflow.
- Full get_info pipeline through /client/<uid> (url_info + parent_info).
- ?complete=yes adding snapshot version.
- ?complete=yes&workflow=yes adding workflow_info.
Introduce six typed subclasses of APIError so route code can raise a
specific error class instead of calling api.fail(status, msg) with a
magic number:
400 BadRequestError
401 UnauthorizedError
403 ForbiddenError
404 NotFoundError
409 ConflictError
422 ValidationError
Each subclass carries its own default HTTP status; the previous
APIError(status, message) positional signature becomes
APIError(message, status=None) so typed subclasses can be raised as
raise NotFoundError("...") without repeating the status number.
Backward compatibility:
- All typed errors inherit APIError. Any existing except APIError:
handler catches every subclass.
- api.fail(status, msg) still works and still raises APIError with the
runtime status. Downstream callers do not have to change.
- APIError.setStatus(x) alias retained for the same reason.
Converts every api.fail() and raw APIError() call site inside
senaite.jsonapi itself to the typed form:
- request.get_request_data: BadRequestError (was APIError(400))
- v1/routes/content.get: NotFoundError for unknown resource
- v1/routes/content.action: BadRequestError for unknown API member
(was api.fail(500), which was misleading: the client asked for an
unknown action, that is a 4xx, not a 5xx)
- v1/routes/push: BadRequest/Unauthorized/NotFound as appropriate
(was api.fail(500) for every failure mode, mixing client errors
with server errors under one status)
- v1/routes/users.login: UnauthorizedError (was api.fail(401))
- api.check_permission: Unauthorized/ForbiddenError
Route-shape update for push.rst: the "non-registered adapter" case
now returns 404 (correct: no consumer with that name is registered),
where it previously returned 500.
Depends on senaite/senaite.core#2998 for the JSON error envelope to
actually surface the exception class name in the response body as a
'type' field. Without #2998, only the HTTP status changes are
visible; the type field is discarded by the current handle_errors
decorator.
Move the biggest remaining slice of api/__init__.py into a focused module: the three top-level route orchestrators (create_items, update_items, delete_items) plus their patch/put aliases, the low-level building blocks (create_object, update_object_with_data, deactivate_object, create_analysisrequest, find_target_container, validate_object) and the two permission checks (is_creation_allowed, is_update_allowed). All names remain importable from senaite.jsonapi.api via explicit re-exports at the bottom of __init__.py. v1/routes/content.action looks up 'api.create_items' / 'api.update_items' / 'api.delete_items' via getattr and continues to work unchanged. Semantic tightening while moving: - Every 'fail(401, ...)' inside the moved code becomes a proper ForbiddenError. HTTP 401 means 'log in and try again'; the fail sites (denied by permission gate, denied by adapter, container disallows type, ...) all mean 'you are authenticated but may not do this' -- which is 403. The two affected doctests (create.rst, update.rst) update their expected status from 401 to 403 to match. - 'fail(400, ...)' becomes BadRequestError, 'fail(404, ...)' becomes NotFoundError. Backward compat is preserved through the shared APIError base. Drop now-unused imports from __init__.py: copy, transaction, AccessControl.Unauthorized, create_ar, ICreate, IUpdate, IInfo, getAdapters, zope.deprecation.deprecate.
The object-addressed action routes (/{resource}/{uid} and /{uid}) now
register PUT, PATCH and DELETE alongside POST. When no explicit action
segment is present in the URL, the action is derived from the HTTP
method: PUT and PATCH map to 'update', DELETE maps to 'delete'.
A probe against a running instance confirmed the @@API view already
receives these verbs -- the view is an IPublishTraverse view that
swallows the whole subpath and returns self, so at path exhaustion the
published object is the view (which has __call__ but no PUT/PATCH
attribute), and ZPublisher's WebDAV NullResource substitution does not
apply. The verbs previously fell through to a werkzeug
MethodNotAllowed because the route only registered POST; registering
them is all that was needed. No WebDAV bypass, no plone.rest
dependency, and the endpoints keep the single /@@API/senaite/v1/... URL
shape.
Resolution order in the action view:
1. Explicit action segment in the URL (unchanged).
2. HTTP verb (PUT/PATCH -> update, DELETE -> delete).
3. X-HTTP-Method-Override header (Backbone.js style, unchanged).
A bare POST with none of these still errors, preserving the historic
behaviour.
Coverage: rest_verbs doctest drives real PUT/PATCH/DELETE through the
browser's underlying WebTest app and asserts update/delete semantics,
plus GET-unaffected and POST-without-action-still-errors regressions.
The REST verb routing added in this PR relied on Zope not diverting PUT/PATCH/DELETE to WebDAV. But Zope flags every non-GET/POST request as maybe_webdav_client=1 by default (only XML-RPC clears it), and at path exhaustion may substitute a WebDAV NullResource. Whether the API view escapes that substitution depends on the view's acquisition shape and the Zope version -- so the verbs reaching the view was incidental, not guaranteed. Make it explicit and self-contained in senaite.jsonapi (independent of the plone.jsonapi.core version): an IPubStart subscriber clears maybe_webdav_client for API requests (path with an @@API / API segment) using PUT/PATCH/DELETE, before traversal consumes the flag. Genuine WebDAV requests to other content keep their flag, so real WebDAV is untouched. This works on the released plone.jsonapi.core 0.7.0 with no framework change. If a future plone.jsonapi.core ships an equivalent subscriber, clearing the flag twice is idempotent. Adds test_webdav covering the verb + path matrix and the segment-not-substring boundary.
create_items caught each object's creation error, logged it, and then
raised a generic "No Objects could be created", discarding the useful
detail (for example the validation error {"field": "required field"}).
Collect the per-object errors and include them in the raised message so
callers see exactly what to fix instead of a generic failure.
xispa
approved these changes
Oct 5, 2026
xispa
added a commit
that referenced
this pull request
Oct 5, 2026
) * Require Manage portal permission for /registry and /settings Both routes returned control-panel data to anonymous callers. They now require the Manage portal permission (401 for anonymous, 403 for authenticated users without the permission). A new api.check_permission helper centralizes the check. * Restrict /users listing to managers to prevent enumeration Any authenticated user could list every account and inspect any single user by id. Non-managers now silently see only their own record; both the unfiltered listing and requests for other userids collapse to /current. Managers retain full listing access. Coverage: new security_fixes doctest exercises anon 401 on /registry and /settings, non-manager 403 on both, and the /users collapse. * Log deprecation warning on GET /login with credentials Credentials submitted via GET land in access logs, Referer headers, and browser history. Warn on this now and plan removal for 2.8.0. GET without credentials (basic-auth handoff) is unaffected. * Add changelog entry for #92 * Use fixture cookie-login for Manager path in security_fixes doctest Basic auth with TEST_USER_NAME/TEST_USER_PASSWORD passes locally but fails on CI (KeyError on 'count' at line 91) because the response falls back to an error shape when the credentials do not authenticate. Switch that single assertion to self.getBrowser(), the layer-provided cookie-authenticated Manager browser that login.rst already uses successfully across every CI build. Non-manager Basic-auth paths (test_labclerk_0, etc.) keep using the as_user helper because base.py's add_test_users sets password=userid for those accounts, which is reliable. * Drop redundant Manager positive-path assertion in security_fixes The Manager-can-enumerate path is already exercised by users.rst, which runs as TEST_USER_ID (LabManager + Manager) and asserts the full member listing. Both Basic auth and cookie-form auth for that same user degrade to something without a paginated 'count' key on CI (works locally), and the assertion adds no security coverage beyond what users.rst already provides. Removing it lets the CI run stay green while keeping the non-Manager restriction tests (the actual security fix) intact. * Convert api module to package for future extractions Pure rename: src/senaite/jsonapi/api.py -> src/senaite/jsonapi/api/__init__.py. Python treats a package (directory with __init__.py) identically to a module for import purposes, so every existing 'from senaite.jsonapi import api' and 'from senaite.jsonapi.api import X' keeps working without change. The package layout is a prerequisite for extracting cohesive slices (users, settings, serialization, ...) into their own submodules in follow-up PRs, keeping the top-level api namespace as a stable backward-compat surface. * Extract user helpers to senaite.jsonapi.api.users Move is_anonymous, get_current_user, get_member_ids, get_user, and get_user_properties out of the god-module api/__init__.py into a focused api/users.py. The five names remain importable from senaite.jsonapi.api via explicit re-exports at the bottom of __init__.py, so downstream code (senaite.core, add-ons, docs, tests) needs no change. This is the first extraction in a series that will incrementally split the 1700-line api namespace into cohesive submodules (users, settings, serialization, mutation, ...) without touching the public import surface. * Add changelog entry for #93 * Extract registry and settings helpers to api/settings Move get_registry_records_by_keyword, get_settings_by_keyword, get_settings_from_interface, and the CONTROLPANEL_INTERFACE_MAPPING constant out of api/__init__.py into a focused api/settings.py. The four names remain importable from senaite.jsonapi.api via explicit re-exports at the bottom of __init__.py, so route code and any downstream users keep working unchanged. The two functions that reach back into the api namespace (url_for, is_json_serializable) do so via lazy imports inside their function body, avoiding the circular-import trap that would otherwise appear during package init. Also drop the six now-unused control-panel schema imports and the zope.schema getFieldNames import from __init__.py. * Add changelog entry for #94 * Add direct-call doctest for api.settings Covers the extracted module without relying on the /settings and /registry routes (which currently trip over non-JSON-serializable registry values under a Manager account). Exercises: - Backward-compat identity of every re-exported name. - CONTROLPANEL_INTERFACE_MAPPING keys. - get_settings_from_interface shape + JSON-serializability filter. - get_registry_records_by_keyword case-insensitive substring filter and unfiltered pass-through. - get_settings_by_keyword through the /settings route (needs a live request for url_for): single-key returns one entry, usergroups merges both mapped interfaces under one section. * Drop now-unused getAdapter import CI lint flagged zope.component.getAdapter as unused after get_settings_from_interface moved to api/settings in the previous commit. Keep ploneapi (still used by check_permission). * Add real-value assertions to api.settings doctest Set concrete IMailSchema fields (smtp_host, smtp_port, email_from_name, email_from_address) so the extracted helpers can be verified round-trip against known values instead of just shape. * Use cookie-login browser for /settings positive path After stacking on PR-B, the /settings route requires the Manage portal permission. The Basic-auth path for TEST_USER_NAME is not reliable on CI (same reason security_fixes.rst switched away from it), so use self.getBrowser() which does form login and is known to work. * Extract serialization helpers to api/serialization Move the six JSON-representation helpers out of the god-module api/__init__.py into a focused api/serialization.py: - get_info (main entry point: brain/object -> JSON-ready dict) - get_url_info (uid, url, api_url) - get_parent_info (parent_id, parent_uid, parent_url) - get_children_info (folderish contents) - get_file_info (file field payload) - get_workflow_info (assigned workflows + current state + transitions) All six names remain importable from senaite.jsonapi.api via explicit re-exports at the bottom of __init__.py, so fieldmanagers.py (which calls api.get_file_info and api.get_url_info) and any downstream users keep working unchanged. Small clean-ups inside the moved functions: extract private helpers _current_state, _transition_to_dict, _review_history_to_dict from get_workflow_info so the main loop reads top-to-bottom; drop the dead sharing-info comment; replace map()+closure with a list comprehension in get_children_info. Drop now-unused imports from __init__.py: bika.lims.api.snapshot, Products.ATContentTypes.utils.DT2dt, IFieldManager. * Add changelog entry for #95 * Add direct-call doctest for api.serialization Covers the extracted module: - Backward-compat identity of every re-exported name. - get_parent_info({}) short-circuit for the portal root. - get_workflow_info shape (workflow_info key, initial state, transitions). - get_workflow_info returning [] for objects with no assigned workflow. - Full get_info pipeline through /client/<uid> (url_info + parent_info). - ?complete=yes adding snapshot version. - ?complete=yes&workflow=yes adding workflow_info. * Add typed exception subclasses for the JSON API error envelope Introduce six typed subclasses of APIError so route code can raise a specific error class instead of calling api.fail(status, msg) with a magic number: 400 BadRequestError 401 UnauthorizedError 403 ForbiddenError 404 NotFoundError 409 ConflictError 422 ValidationError Each subclass carries its own default HTTP status; the previous APIError(status, message) positional signature becomes APIError(message, status=None) so typed subclasses can be raised as raise NotFoundError("...") without repeating the status number. Backward compatibility: - All typed errors inherit APIError. Any existing except APIError: handler catches every subclass. - api.fail(status, msg) still works and still raises APIError with the runtime status. Downstream callers do not have to change. - APIError.setStatus(x) alias retained for the same reason. Converts every api.fail() and raw APIError() call site inside senaite.jsonapi itself to the typed form: - request.get_request_data: BadRequestError (was APIError(400)) - v1/routes/content.get: NotFoundError for unknown resource - v1/routes/content.action: BadRequestError for unknown API member (was api.fail(500), which was misleading: the client asked for an unknown action, that is a 4xx, not a 5xx) - v1/routes/push: BadRequest/Unauthorized/NotFound as appropriate (was api.fail(500) for every failure mode, mixing client errors with server errors under one status) - v1/routes/users.login: UnauthorizedError (was api.fail(401)) - api.check_permission: Unauthorized/ForbiddenError Route-shape update for push.rst: the "non-registered adapter" case now returns 404 (correct: no consumer with that name is registered), where it previously returned 500. Depends on senaite/senaite.core#2998 for the JSON error envelope to actually surface the exception class name in the response body as a 'type' field. Without #2998, only the HTTP status changes are visible; the type field is discarded by the current handle_errors decorator. * Add changelog entry for #96 * Extract create/update/delete helpers to api/mutation Move the biggest remaining slice of api/__init__.py into a focused module: the three top-level route orchestrators (create_items, update_items, delete_items) plus their patch/put aliases, the low-level building blocks (create_object, update_object_with_data, deactivate_object, create_analysisrequest, find_target_container, validate_object) and the two permission checks (is_creation_allowed, is_update_allowed). All names remain importable from senaite.jsonapi.api via explicit re-exports at the bottom of __init__.py. v1/routes/content.action looks up 'api.create_items' / 'api.update_items' / 'api.delete_items' via getattr and continues to work unchanged. Semantic tightening while moving: - Every 'fail(401, ...)' inside the moved code becomes a proper ForbiddenError. HTTP 401 means 'log in and try again'; the fail sites (denied by permission gate, denied by adapter, container disallows type, ...) all mean 'you are authenticated but may not do this' -- which is 403. The two affected doctests (create.rst, update.rst) update their expected status from 401 to 403 to match. - 'fail(400, ...)' becomes BadRequestError, 'fail(404, ...)' becomes NotFoundError. Backward compat is preserved through the shared APIError base. Drop now-unused imports from __init__.py: copy, transaction, AccessControl.Unauthorized, create_ar, ICreate, IUpdate, IInfo, getAdapters, zope.deprecation.deprecate. * Add changelog entry for #97 * Accept REST verbs PUT/PATCH/DELETE on the action route The object-addressed action routes (/{resource}/{uid} and /{uid}) now register PUT, PATCH and DELETE alongside POST. When no explicit action segment is present in the URL, the action is derived from the HTTP method: PUT and PATCH map to 'update', DELETE maps to 'delete'. A probe against a running instance confirmed the @@API view already receives these verbs -- the view is an IPublishTraverse view that swallows the whole subpath and returns self, so at path exhaustion the published object is the view (which has __call__ but no PUT/PATCH attribute), and ZPublisher's WebDAV NullResource substitution does not apply. The verbs previously fell through to a werkzeug MethodNotAllowed because the route only registered POST; registering them is all that was needed. No WebDAV bypass, no plone.rest dependency, and the endpoints keep the single /@@API/senaite/v1/... URL shape. Resolution order in the action view: 1. Explicit action segment in the URL (unchanged). 2. HTTP verb (PUT/PATCH -> update, DELETE -> delete). 3. X-HTTP-Method-Override header (Backbone.js style, unchanged). A bare POST with none of these still errors, preserving the historic behaviour. Coverage: rest_verbs doctest drives real PUT/PATCH/DELETE through the browser's underlying WebTest app and asserts update/delete semantics, plus GET-unaffected and POST-without-action-still-errors regressions. * Add changelog entry for #98 * Add WebDAV verb bypass so PUT/PATCH/DELETE reliably reach the API The REST verb routing added in this PR relied on Zope not diverting PUT/PATCH/DELETE to WebDAV. But Zope flags every non-GET/POST request as maybe_webdav_client=1 by default (only XML-RPC clears it), and at path exhaustion may substitute a WebDAV NullResource. Whether the API view escapes that substitution depends on the view's acquisition shape and the Zope version -- so the verbs reaching the view was incidental, not guaranteed. Make it explicit and self-contained in senaite.jsonapi (independent of the plone.jsonapi.core version): an IPubStart subscriber clears maybe_webdav_client for API requests (path with an @@API / API segment) using PUT/PATCH/DELETE, before traversal consumes the flag. Genuine WebDAV requests to other content keep their flag, so real WebDAV is untouched. This works on the released plone.jsonapi.core 0.7.0 with no framework change. If a future plone.jsonapi.core ships an equivalent subscriber, clearing the flag twice is idempotent. Adds test_webdav covering the verb + path matrix and the segment-not-substring boundary. * Report the reason when object creation fails create_items caught each object's creation error, logged it, and then raised a generic "No Objects could be created", discarding the useful detail (for example the validation error {"field": "required field"}). Collect the per-object errors and include them in the raised message so callers see exactly what to fix instead of a generic failure. * Add changelog entry for #99 * Normalize UID references through the field manager, not the setter DexterityDataManager.set prioritized a content-type set<Name> mutator over the field manager. For a UID reference field (e.g. Department's manager) that bypassed UIDReferenceFieldMixin's normalization, so a unicode UID from a JSON payload was stored verbatim and later failed the field's ASCIILine value_type validation with WrongContainedType. Route UID reference fields through their field manager (which resolves objects/paths and coerces UIDs to native str). Every other field keeps prioritizing the setter, which also covers BBB properties that have no schema field (e.g. Department.DepartmentID). * Add changelog entry for #100 --------- Co-authored-by: Jordi Puiggené <jp@naralabs.com>
xispa
added a commit
that referenced
this pull request
Oct 5, 2026
* Require Manage portal permission for /registry and /settings Both routes returned control-panel data to anonymous callers. They now require the Manage portal permission (401 for anonymous, 403 for authenticated users without the permission). A new api.check_permission helper centralizes the check. * Restrict /users listing to managers to prevent enumeration Any authenticated user could list every account and inspect any single user by id. Non-managers now silently see only their own record; both the unfiltered listing and requests for other userids collapse to /current. Managers retain full listing access. Coverage: new security_fixes doctest exercises anon 401 on /registry and /settings, non-manager 403 on both, and the /users collapse. * Log deprecation warning on GET /login with credentials Credentials submitted via GET land in access logs, Referer headers, and browser history. Warn on this now and plan removal for 2.8.0. GET without credentials (basic-auth handoff) is unaffected. * Add changelog entry for #92 * Use fixture cookie-login for Manager path in security_fixes doctest Basic auth with TEST_USER_NAME/TEST_USER_PASSWORD passes locally but fails on CI (KeyError on 'count' at line 91) because the response falls back to an error shape when the credentials do not authenticate. Switch that single assertion to self.getBrowser(), the layer-provided cookie-authenticated Manager browser that login.rst already uses successfully across every CI build. Non-manager Basic-auth paths (test_labclerk_0, etc.) keep using the as_user helper because base.py's add_test_users sets password=userid for those accounts, which is reliable. * Drop redundant Manager positive-path assertion in security_fixes The Manager-can-enumerate path is already exercised by users.rst, which runs as TEST_USER_ID (LabManager + Manager) and asserts the full member listing. Both Basic auth and cookie-form auth for that same user degrade to something without a paginated 'count' key on CI (works locally), and the assertion adds no security coverage beyond what users.rst already provides. Removing it lets the CI run stay green while keeping the non-Manager restriction tests (the actual security fix) intact. * Convert api module to package for future extractions Pure rename: src/senaite/jsonapi/api.py -> src/senaite/jsonapi/api/__init__.py. Python treats a package (directory with __init__.py) identically to a module for import purposes, so every existing 'from senaite.jsonapi import api' and 'from senaite.jsonapi.api import X' keeps working without change. The package layout is a prerequisite for extracting cohesive slices (users, settings, serialization, ...) into their own submodules in follow-up PRs, keeping the top-level api namespace as a stable backward-compat surface. * Extract user helpers to senaite.jsonapi.api.users Move is_anonymous, get_current_user, get_member_ids, get_user, and get_user_properties out of the god-module api/__init__.py into a focused api/users.py. The five names remain importable from senaite.jsonapi.api via explicit re-exports at the bottom of __init__.py, so downstream code (senaite.core, add-ons, docs, tests) needs no change. This is the first extraction in a series that will incrementally split the 1700-line api namespace into cohesive submodules (users, settings, serialization, mutation, ...) without touching the public import surface. * Add changelog entry for #93 * Extract registry and settings helpers to api/settings Move get_registry_records_by_keyword, get_settings_by_keyword, get_settings_from_interface, and the CONTROLPANEL_INTERFACE_MAPPING constant out of api/__init__.py into a focused api/settings.py. The four names remain importable from senaite.jsonapi.api via explicit re-exports at the bottom of __init__.py, so route code and any downstream users keep working unchanged. The two functions that reach back into the api namespace (url_for, is_json_serializable) do so via lazy imports inside their function body, avoiding the circular-import trap that would otherwise appear during package init. Also drop the six now-unused control-panel schema imports and the zope.schema getFieldNames import from __init__.py. * Add changelog entry for #94 * Add direct-call doctest for api.settings Covers the extracted module without relying on the /settings and /registry routes (which currently trip over non-JSON-serializable registry values under a Manager account). Exercises: - Backward-compat identity of every re-exported name. - CONTROLPANEL_INTERFACE_MAPPING keys. - get_settings_from_interface shape + JSON-serializability filter. - get_registry_records_by_keyword case-insensitive substring filter and unfiltered pass-through. - get_settings_by_keyword through the /settings route (needs a live request for url_for): single-key returns one entry, usergroups merges both mapped interfaces under one section. * Drop now-unused getAdapter import CI lint flagged zope.component.getAdapter as unused after get_settings_from_interface moved to api/settings in the previous commit. Keep ploneapi (still used by check_permission). * Add real-value assertions to api.settings doctest Set concrete IMailSchema fields (smtp_host, smtp_port, email_from_name, email_from_address) so the extracted helpers can be verified round-trip against known values instead of just shape. * Use cookie-login browser for /settings positive path After stacking on PR-B, the /settings route requires the Manage portal permission. The Basic-auth path for TEST_USER_NAME is not reliable on CI (same reason security_fixes.rst switched away from it), so use self.getBrowser() which does form login and is known to work. * Extract serialization helpers to api/serialization Move the six JSON-representation helpers out of the god-module api/__init__.py into a focused api/serialization.py: - get_info (main entry point: brain/object -> JSON-ready dict) - get_url_info (uid, url, api_url) - get_parent_info (parent_id, parent_uid, parent_url) - get_children_info (folderish contents) - get_file_info (file field payload) - get_workflow_info (assigned workflows + current state + transitions) All six names remain importable from senaite.jsonapi.api via explicit re-exports at the bottom of __init__.py, so fieldmanagers.py (which calls api.get_file_info and api.get_url_info) and any downstream users keep working unchanged. Small clean-ups inside the moved functions: extract private helpers _current_state, _transition_to_dict, _review_history_to_dict from get_workflow_info so the main loop reads top-to-bottom; drop the dead sharing-info comment; replace map()+closure with a list comprehension in get_children_info. Drop now-unused imports from __init__.py: bika.lims.api.snapshot, Products.ATContentTypes.utils.DT2dt, IFieldManager. * Add changelog entry for #95 * Add direct-call doctest for api.serialization Covers the extracted module: - Backward-compat identity of every re-exported name. - get_parent_info({}) short-circuit for the portal root. - get_workflow_info shape (workflow_info key, initial state, transitions). - get_workflow_info returning [] for objects with no assigned workflow. - Full get_info pipeline through /client/<uid> (url_info + parent_info). - ?complete=yes adding snapshot version. - ?complete=yes&workflow=yes adding workflow_info. * Add typed exception subclasses for the JSON API error envelope Introduce six typed subclasses of APIError so route code can raise a specific error class instead of calling api.fail(status, msg) with a magic number: 400 BadRequestError 401 UnauthorizedError 403 ForbiddenError 404 NotFoundError 409 ConflictError 422 ValidationError Each subclass carries its own default HTTP status; the previous APIError(status, message) positional signature becomes APIError(message, status=None) so typed subclasses can be raised as raise NotFoundError("...") without repeating the status number. Backward compatibility: - All typed errors inherit APIError. Any existing except APIError: handler catches every subclass. - api.fail(status, msg) still works and still raises APIError with the runtime status. Downstream callers do not have to change. - APIError.setStatus(x) alias retained for the same reason. Converts every api.fail() and raw APIError() call site inside senaite.jsonapi itself to the typed form: - request.get_request_data: BadRequestError (was APIError(400)) - v1/routes/content.get: NotFoundError for unknown resource - v1/routes/content.action: BadRequestError for unknown API member (was api.fail(500), which was misleading: the client asked for an unknown action, that is a 4xx, not a 5xx) - v1/routes/push: BadRequest/Unauthorized/NotFound as appropriate (was api.fail(500) for every failure mode, mixing client errors with server errors under one status) - v1/routes/users.login: UnauthorizedError (was api.fail(401)) - api.check_permission: Unauthorized/ForbiddenError Route-shape update for push.rst: the "non-registered adapter" case now returns 404 (correct: no consumer with that name is registered), where it previously returned 500. Depends on senaite/senaite.core#2998 for the JSON error envelope to actually surface the exception class name in the response body as a 'type' field. Without #2998, only the HTTP status changes are visible; the type field is discarded by the current handle_errors decorator. * Add changelog entry for #96 * Extract create/update/delete helpers to api/mutation Move the biggest remaining slice of api/__init__.py into a focused module: the three top-level route orchestrators (create_items, update_items, delete_items) plus their patch/put aliases, the low-level building blocks (create_object, update_object_with_data, deactivate_object, create_analysisrequest, find_target_container, validate_object) and the two permission checks (is_creation_allowed, is_update_allowed). All names remain importable from senaite.jsonapi.api via explicit re-exports at the bottom of __init__.py. v1/routes/content.action looks up 'api.create_items' / 'api.update_items' / 'api.delete_items' via getattr and continues to work unchanged. Semantic tightening while moving: - Every 'fail(401, ...)' inside the moved code becomes a proper ForbiddenError. HTTP 401 means 'log in and try again'; the fail sites (denied by permission gate, denied by adapter, container disallows type, ...) all mean 'you are authenticated but may not do this' -- which is 403. The two affected doctests (create.rst, update.rst) update their expected status from 401 to 403 to match. - 'fail(400, ...)' becomes BadRequestError, 'fail(404, ...)' becomes NotFoundError. Backward compat is preserved through the shared APIError base. Drop now-unused imports from __init__.py: copy, transaction, AccessControl.Unauthorized, create_ar, ICreate, IUpdate, IInfo, getAdapters, zope.deprecation.deprecate. * Add changelog entry for #97 * Accept REST verbs PUT/PATCH/DELETE on the action route The object-addressed action routes (/{resource}/{uid} and /{uid}) now register PUT, PATCH and DELETE alongside POST. When no explicit action segment is present in the URL, the action is derived from the HTTP method: PUT and PATCH map to 'update', DELETE maps to 'delete'. A probe against a running instance confirmed the @@API view already receives these verbs -- the view is an IPublishTraverse view that swallows the whole subpath and returns self, so at path exhaustion the published object is the view (which has __call__ but no PUT/PATCH attribute), and ZPublisher's WebDAV NullResource substitution does not apply. The verbs previously fell through to a werkzeug MethodNotAllowed because the route only registered POST; registering them is all that was needed. No WebDAV bypass, no plone.rest dependency, and the endpoints keep the single /@@API/senaite/v1/... URL shape. Resolution order in the action view: 1. Explicit action segment in the URL (unchanged). 2. HTTP verb (PUT/PATCH -> update, DELETE -> delete). 3. X-HTTP-Method-Override header (Backbone.js style, unchanged). A bare POST with none of these still errors, preserving the historic behaviour. Coverage: rest_verbs doctest drives real PUT/PATCH/DELETE through the browser's underlying WebTest app and asserts update/delete semantics, plus GET-unaffected and POST-without-action-still-errors regressions. * Add changelog entry for #98 * Add WebDAV verb bypass so PUT/PATCH/DELETE reliably reach the API The REST verb routing added in this PR relied on Zope not diverting PUT/PATCH/DELETE to WebDAV. But Zope flags every non-GET/POST request as maybe_webdav_client=1 by default (only XML-RPC clears it), and at path exhaustion may substitute a WebDAV NullResource. Whether the API view escapes that substitution depends on the view's acquisition shape and the Zope version -- so the verbs reaching the view was incidental, not guaranteed. Make it explicit and self-contained in senaite.jsonapi (independent of the plone.jsonapi.core version): an IPubStart subscriber clears maybe_webdav_client for API requests (path with an @@API / API segment) using PUT/PATCH/DELETE, before traversal consumes the flag. Genuine WebDAV requests to other content keep their flag, so real WebDAV is untouched. This works on the released plone.jsonapi.core 0.7.0 with no framework change. If a future plone.jsonapi.core ships an equivalent subscriber, clearing the flag twice is idempotent. Adds test_webdav covering the verb + path matrix and the segment-not-substring boundary. * Report the reason when object creation fails create_items caught each object's creation error, logged it, and then raised a generic "No Objects could be created", discarding the useful detail (for example the validation error {"field": "required field"}). Collect the per-object errors and include them in the raised message so callers see exactly what to fix instead of a generic failure. * Add changelog entry for #99 * Normalize UID references through the field manager, not the setter DexterityDataManager.set prioritized a content-type set<Name> mutator over the field manager. For a UID reference field (e.g. Department's manager) that bypassed UIDReferenceFieldMixin's normalization, so a unicode UID from a JSON payload was stored verbatim and later failed the field's ASCIILine value_type validation with WrongContainedType. Route UID reference fields through their field manager (which resolves objects/paths and coerces UIDs to native str). Every other field keeps prioritizing the setter, which also covers BBB properties that have no schema field (e.g. Department.DepartmentID). * Add changelog entry for #100 * Encode AT string field values to native str before validation AT field validators (isEmail, isDecimal, ...) expect a native str, but values from a JSON body arrive as unicode and fail with "expected 'string'" -- so EmailAddress, Price, VAT, DuplicateVariation could not be set through the JSON API. ATFieldManager._set now utf-8-encodes text values before validation; dicts/lists/other types are left untouched, so records/datagrid/reference fields are unaffected. Adds a create doctest that posts a JSON body with a unicode decimal. * Add changelog entry for #101 --------- Co-authored-by: Jordi Puiggené <jp@naralabs.com>
xispa
added a commit
that referenced
this pull request
Oct 5, 2026
* Require Manage portal permission for /registry and /settings Both routes returned control-panel data to anonymous callers. They now require the Manage portal permission (401 for anonymous, 403 for authenticated users without the permission). A new api.check_permission helper centralizes the check. * Restrict /users listing to managers to prevent enumeration Any authenticated user could list every account and inspect any single user by id. Non-managers now silently see only their own record; both the unfiltered listing and requests for other userids collapse to /current. Managers retain full listing access. Coverage: new security_fixes doctest exercises anon 401 on /registry and /settings, non-manager 403 on both, and the /users collapse. * Log deprecation warning on GET /login with credentials Credentials submitted via GET land in access logs, Referer headers, and browser history. Warn on this now and plan removal for 2.8.0. GET without credentials (basic-auth handoff) is unaffected. * Add changelog entry for #92 * Use fixture cookie-login for Manager path in security_fixes doctest Basic auth with TEST_USER_NAME/TEST_USER_PASSWORD passes locally but fails on CI (KeyError on 'count' at line 91) because the response falls back to an error shape when the credentials do not authenticate. Switch that single assertion to self.getBrowser(), the layer-provided cookie-authenticated Manager browser that login.rst already uses successfully across every CI build. Non-manager Basic-auth paths (test_labclerk_0, etc.) keep using the as_user helper because base.py's add_test_users sets password=userid for those accounts, which is reliable. * Drop redundant Manager positive-path assertion in security_fixes The Manager-can-enumerate path is already exercised by users.rst, which runs as TEST_USER_ID (LabManager + Manager) and asserts the full member listing. Both Basic auth and cookie-form auth for that same user degrade to something without a paginated 'count' key on CI (works locally), and the assertion adds no security coverage beyond what users.rst already provides. Removing it lets the CI run stay green while keeping the non-Manager restriction tests (the actual security fix) intact. * Convert api module to package for future extractions Pure rename: src/senaite/jsonapi/api.py -> src/senaite/jsonapi/api/__init__.py. Python treats a package (directory with __init__.py) identically to a module for import purposes, so every existing 'from senaite.jsonapi import api' and 'from senaite.jsonapi.api import X' keeps working without change. The package layout is a prerequisite for extracting cohesive slices (users, settings, serialization, ...) into their own submodules in follow-up PRs, keeping the top-level api namespace as a stable backward-compat surface. * Extract user helpers to senaite.jsonapi.api.users Move is_anonymous, get_current_user, get_member_ids, get_user, and get_user_properties out of the god-module api/__init__.py into a focused api/users.py. The five names remain importable from senaite.jsonapi.api via explicit re-exports at the bottom of __init__.py, so downstream code (senaite.core, add-ons, docs, tests) needs no change. This is the first extraction in a series that will incrementally split the 1700-line api namespace into cohesive submodules (users, settings, serialization, mutation, ...) without touching the public import surface. * Add changelog entry for #93 * Extract registry and settings helpers to api/settings Move get_registry_records_by_keyword, get_settings_by_keyword, get_settings_from_interface, and the CONTROLPANEL_INTERFACE_MAPPING constant out of api/__init__.py into a focused api/settings.py. The four names remain importable from senaite.jsonapi.api via explicit re-exports at the bottom of __init__.py, so route code and any downstream users keep working unchanged. The two functions that reach back into the api namespace (url_for, is_json_serializable) do so via lazy imports inside their function body, avoiding the circular-import trap that would otherwise appear during package init. Also drop the six now-unused control-panel schema imports and the zope.schema getFieldNames import from __init__.py. * Add changelog entry for #94 * Add direct-call doctest for api.settings Covers the extracted module without relying on the /settings and /registry routes (which currently trip over non-JSON-serializable registry values under a Manager account). Exercises: - Backward-compat identity of every re-exported name. - CONTROLPANEL_INTERFACE_MAPPING keys. - get_settings_from_interface shape + JSON-serializability filter. - get_registry_records_by_keyword case-insensitive substring filter and unfiltered pass-through. - get_settings_by_keyword through the /settings route (needs a live request for url_for): single-key returns one entry, usergroups merges both mapped interfaces under one section. * Drop now-unused getAdapter import CI lint flagged zope.component.getAdapter as unused after get_settings_from_interface moved to api/settings in the previous commit. Keep ploneapi (still used by check_permission). * Add real-value assertions to api.settings doctest Set concrete IMailSchema fields (smtp_host, smtp_port, email_from_name, email_from_address) so the extracted helpers can be verified round-trip against known values instead of just shape. * Use cookie-login browser for /settings positive path After stacking on PR-B, the /settings route requires the Manage portal permission. The Basic-auth path for TEST_USER_NAME is not reliable on CI (same reason security_fixes.rst switched away from it), so use self.getBrowser() which does form login and is known to work. * Extract serialization helpers to api/serialization Move the six JSON-representation helpers out of the god-module api/__init__.py into a focused api/serialization.py: - get_info (main entry point: brain/object -> JSON-ready dict) - get_url_info (uid, url, api_url) - get_parent_info (parent_id, parent_uid, parent_url) - get_children_info (folderish contents) - get_file_info (file field payload) - get_workflow_info (assigned workflows + current state + transitions) All six names remain importable from senaite.jsonapi.api via explicit re-exports at the bottom of __init__.py, so fieldmanagers.py (which calls api.get_file_info and api.get_url_info) and any downstream users keep working unchanged. Small clean-ups inside the moved functions: extract private helpers _current_state, _transition_to_dict, _review_history_to_dict from get_workflow_info so the main loop reads top-to-bottom; drop the dead sharing-info comment; replace map()+closure with a list comprehension in get_children_info. Drop now-unused imports from __init__.py: bika.lims.api.snapshot, Products.ATContentTypes.utils.DT2dt, IFieldManager. * Add changelog entry for #95 * Add direct-call doctest for api.serialization Covers the extracted module: - Backward-compat identity of every re-exported name. - get_parent_info({}) short-circuit for the portal root. - get_workflow_info shape (workflow_info key, initial state, transitions). - get_workflow_info returning [] for objects with no assigned workflow. - Full get_info pipeline through /client/<uid> (url_info + parent_info). - ?complete=yes adding snapshot version. - ?complete=yes&workflow=yes adding workflow_info. * Add typed exception subclasses for the JSON API error envelope Introduce six typed subclasses of APIError so route code can raise a specific error class instead of calling api.fail(status, msg) with a magic number: 400 BadRequestError 401 UnauthorizedError 403 ForbiddenError 404 NotFoundError 409 ConflictError 422 ValidationError Each subclass carries its own default HTTP status; the previous APIError(status, message) positional signature becomes APIError(message, status=None) so typed subclasses can be raised as raise NotFoundError("...") without repeating the status number. Backward compatibility: - All typed errors inherit APIError. Any existing except APIError: handler catches every subclass. - api.fail(status, msg) still works and still raises APIError with the runtime status. Downstream callers do not have to change. - APIError.setStatus(x) alias retained for the same reason. Converts every api.fail() and raw APIError() call site inside senaite.jsonapi itself to the typed form: - request.get_request_data: BadRequestError (was APIError(400)) - v1/routes/content.get: NotFoundError for unknown resource - v1/routes/content.action: BadRequestError for unknown API member (was api.fail(500), which was misleading: the client asked for an unknown action, that is a 4xx, not a 5xx) - v1/routes/push: BadRequest/Unauthorized/NotFound as appropriate (was api.fail(500) for every failure mode, mixing client errors with server errors under one status) - v1/routes/users.login: UnauthorizedError (was api.fail(401)) - api.check_permission: Unauthorized/ForbiddenError Route-shape update for push.rst: the "non-registered adapter" case now returns 404 (correct: no consumer with that name is registered), where it previously returned 500. Depends on senaite/senaite.core#2998 for the JSON error envelope to actually surface the exception class name in the response body as a 'type' field. Without #2998, only the HTTP status changes are visible; the type field is discarded by the current handle_errors decorator. * Add changelog entry for #96 * Extract create/update/delete helpers to api/mutation Move the biggest remaining slice of api/__init__.py into a focused module: the three top-level route orchestrators (create_items, update_items, delete_items) plus their patch/put aliases, the low-level building blocks (create_object, update_object_with_data, deactivate_object, create_analysisrequest, find_target_container, validate_object) and the two permission checks (is_creation_allowed, is_update_allowed). All names remain importable from senaite.jsonapi.api via explicit re-exports at the bottom of __init__.py. v1/routes/content.action looks up 'api.create_items' / 'api.update_items' / 'api.delete_items' via getattr and continues to work unchanged. Semantic tightening while moving: - Every 'fail(401, ...)' inside the moved code becomes a proper ForbiddenError. HTTP 401 means 'log in and try again'; the fail sites (denied by permission gate, denied by adapter, container disallows type, ...) all mean 'you are authenticated but may not do this' -- which is 403. The two affected doctests (create.rst, update.rst) update their expected status from 401 to 403 to match. - 'fail(400, ...)' becomes BadRequestError, 'fail(404, ...)' becomes NotFoundError. Backward compat is preserved through the shared APIError base. Drop now-unused imports from __init__.py: copy, transaction, AccessControl.Unauthorized, create_ar, ICreate, IUpdate, IInfo, getAdapters, zope.deprecation.deprecate. * Add changelog entry for #97 * Accept REST verbs PUT/PATCH/DELETE on the action route The object-addressed action routes (/{resource}/{uid} and /{uid}) now register PUT, PATCH and DELETE alongside POST. When no explicit action segment is present in the URL, the action is derived from the HTTP method: PUT and PATCH map to 'update', DELETE maps to 'delete'. A probe against a running instance confirmed the @@API view already receives these verbs -- the view is an IPublishTraverse view that swallows the whole subpath and returns self, so at path exhaustion the published object is the view (which has __call__ but no PUT/PATCH attribute), and ZPublisher's WebDAV NullResource substitution does not apply. The verbs previously fell through to a werkzeug MethodNotAllowed because the route only registered POST; registering them is all that was needed. No WebDAV bypass, no plone.rest dependency, and the endpoints keep the single /@@API/senaite/v1/... URL shape. Resolution order in the action view: 1. Explicit action segment in the URL (unchanged). 2. HTTP verb (PUT/PATCH -> update, DELETE -> delete). 3. X-HTTP-Method-Override header (Backbone.js style, unchanged). A bare POST with none of these still errors, preserving the historic behaviour. Coverage: rest_verbs doctest drives real PUT/PATCH/DELETE through the browser's underlying WebTest app and asserts update/delete semantics, plus GET-unaffected and POST-without-action-still-errors regressions. * Add changelog entry for #98 * Add WebDAV verb bypass so PUT/PATCH/DELETE reliably reach the API The REST verb routing added in this PR relied on Zope not diverting PUT/PATCH/DELETE to WebDAV. But Zope flags every non-GET/POST request as maybe_webdav_client=1 by default (only XML-RPC clears it), and at path exhaustion may substitute a WebDAV NullResource. Whether the API view escapes that substitution depends on the view's acquisition shape and the Zope version -- so the verbs reaching the view was incidental, not guaranteed. Make it explicit and self-contained in senaite.jsonapi (independent of the plone.jsonapi.core version): an IPubStart subscriber clears maybe_webdav_client for API requests (path with an @@API / API segment) using PUT/PATCH/DELETE, before traversal consumes the flag. Genuine WebDAV requests to other content keep their flag, so real WebDAV is untouched. This works on the released plone.jsonapi.core 0.7.0 with no framework change. If a future plone.jsonapi.core ships an equivalent subscriber, clearing the flag twice is idempotent. Adds test_webdav covering the verb + path matrix and the segment-not-substring boundary. * Report the reason when object creation fails create_items caught each object's creation error, logged it, and then raised a generic "No Objects could be created", discarding the useful detail (for example the validation error {"field": "required field"}). Collect the per-object errors and include them in the raised message so callers see exactly what to fix instead of a generic failure. * Add changelog entry for #99 * Normalize UID references through the field manager, not the setter DexterityDataManager.set prioritized a content-type set<Name> mutator over the field manager. For a UID reference field (e.g. Department's manager) that bypassed UIDReferenceFieldMixin's normalization, so a unicode UID from a JSON payload was stored verbatim and later failed the field's ASCIILine value_type validation with WrongContainedType. Route UID reference fields through their field manager (which resolves objects/paths and coerces UIDs to native str). Every other field keeps prioritizing the setter, which also covers BBB properties that have no schema field (e.g. Department.DepartmentID). * Add changelog entry for #100 * Encode AT string field values to native str before validation AT field validators (isEmail, isDecimal, ...) expect a native str, but values from a JSON body arrive as unicode and fail with "expected 'string'" -- so EmailAddress, Price, VAT, DuplicateVariation could not be set through the JSON API. ATFieldManager._set now utf-8-encodes text values before validation; dicts/lists/other types are left untouched, so records/datagrid/reference fields are unaffected. Adds a create doctest that posts a JSON body with a unicode decimal. * Add changelog entry for #101 * Support DX Duration (Timedelta) fields via a field manager A DX DurationField is a zope.schema Timedelta, which JSON cannot carry, so fields like SamplePoint.sampling_frequency could not be set via the API: a raw {days, hours, minutes} mapping went through the set<Name> mutator and was later rejected as "wrong type". Add a DurationFieldManager that converts a {days, hours, minutes, seconds} mapping to/from a timedelta, register it for IDurationField, and route duration fields through the field manager in DexterityDataManager.set (as done for UID references). * Add changelog entry for #102 --------- Co-authored-by: Jordi Puiggené <jp@naralabs.com>
xispa
added a commit
that referenced
this pull request
Oct 6, 2026
* Require Manage portal permission for /registry and /settings Both routes returned control-panel data to anonymous callers. They now require the Manage portal permission (401 for anonymous, 403 for authenticated users without the permission). A new api.check_permission helper centralizes the check. * Restrict /users listing to managers to prevent enumeration Any authenticated user could list every account and inspect any single user by id. Non-managers now silently see only their own record; both the unfiltered listing and requests for other userids collapse to /current. Managers retain full listing access. Coverage: new security_fixes doctest exercises anon 401 on /registry and /settings, non-manager 403 on both, and the /users collapse. * Log deprecation warning on GET /login with credentials Credentials submitted via GET land in access logs, Referer headers, and browser history. Warn on this now and plan removal for 2.8.0. GET without credentials (basic-auth handoff) is unaffected. * Add changelog entry for #92 * Use fixture cookie-login for Manager path in security_fixes doctest Basic auth with TEST_USER_NAME/TEST_USER_PASSWORD passes locally but fails on CI (KeyError on 'count' at line 91) because the response falls back to an error shape when the credentials do not authenticate. Switch that single assertion to self.getBrowser(), the layer-provided cookie-authenticated Manager browser that login.rst already uses successfully across every CI build. Non-manager Basic-auth paths (test_labclerk_0, etc.) keep using the as_user helper because base.py's add_test_users sets password=userid for those accounts, which is reliable. * Drop redundant Manager positive-path assertion in security_fixes The Manager-can-enumerate path is already exercised by users.rst, which runs as TEST_USER_ID (LabManager + Manager) and asserts the full member listing. Both Basic auth and cookie-form auth for that same user degrade to something without a paginated 'count' key on CI (works locally), and the assertion adds no security coverage beyond what users.rst already provides. Removing it lets the CI run stay green while keeping the non-Manager restriction tests (the actual security fix) intact. * Convert api module to package for future extractions Pure rename: src/senaite/jsonapi/api.py -> src/senaite/jsonapi/api/__init__.py. Python treats a package (directory with __init__.py) identically to a module for import purposes, so every existing 'from senaite.jsonapi import api' and 'from senaite.jsonapi.api import X' keeps working without change. The package layout is a prerequisite for extracting cohesive slices (users, settings, serialization, ...) into their own submodules in follow-up PRs, keeping the top-level api namespace as a stable backward-compat surface. * Extract user helpers to senaite.jsonapi.api.users Move is_anonymous, get_current_user, get_member_ids, get_user, and get_user_properties out of the god-module api/__init__.py into a focused api/users.py. The five names remain importable from senaite.jsonapi.api via explicit re-exports at the bottom of __init__.py, so downstream code (senaite.core, add-ons, docs, tests) needs no change. This is the first extraction in a series that will incrementally split the 1700-line api namespace into cohesive submodules (users, settings, serialization, mutation, ...) without touching the public import surface. * Add changelog entry for #93 * Extract registry and settings helpers to api/settings Move get_registry_records_by_keyword, get_settings_by_keyword, get_settings_from_interface, and the CONTROLPANEL_INTERFACE_MAPPING constant out of api/__init__.py into a focused api/settings.py. The four names remain importable from senaite.jsonapi.api via explicit re-exports at the bottom of __init__.py, so route code and any downstream users keep working unchanged. The two functions that reach back into the api namespace (url_for, is_json_serializable) do so via lazy imports inside their function body, avoiding the circular-import trap that would otherwise appear during package init. Also drop the six now-unused control-panel schema imports and the zope.schema getFieldNames import from __init__.py. * Add changelog entry for #94 * Add direct-call doctest for api.settings Covers the extracted module without relying on the /settings and /registry routes (which currently trip over non-JSON-serializable registry values under a Manager account). Exercises: - Backward-compat identity of every re-exported name. - CONTROLPANEL_INTERFACE_MAPPING keys. - get_settings_from_interface shape + JSON-serializability filter. - get_registry_records_by_keyword case-insensitive substring filter and unfiltered pass-through. - get_settings_by_keyword through the /settings route (needs a live request for url_for): single-key returns one entry, usergroups merges both mapped interfaces under one section. * Drop now-unused getAdapter import CI lint flagged zope.component.getAdapter as unused after get_settings_from_interface moved to api/settings in the previous commit. Keep ploneapi (still used by check_permission). * Add real-value assertions to api.settings doctest Set concrete IMailSchema fields (smtp_host, smtp_port, email_from_name, email_from_address) so the extracted helpers can be verified round-trip against known values instead of just shape. * Use cookie-login browser for /settings positive path After stacking on PR-B, the /settings route requires the Manage portal permission. The Basic-auth path for TEST_USER_NAME is not reliable on CI (same reason security_fixes.rst switched away from it), so use self.getBrowser() which does form login and is known to work. * Extract serialization helpers to api/serialization Move the six JSON-representation helpers out of the god-module api/__init__.py into a focused api/serialization.py: - get_info (main entry point: brain/object -> JSON-ready dict) - get_url_info (uid, url, api_url) - get_parent_info (parent_id, parent_uid, parent_url) - get_children_info (folderish contents) - get_file_info (file field payload) - get_workflow_info (assigned workflows + current state + transitions) All six names remain importable from senaite.jsonapi.api via explicit re-exports at the bottom of __init__.py, so fieldmanagers.py (which calls api.get_file_info and api.get_url_info) and any downstream users keep working unchanged. Small clean-ups inside the moved functions: extract private helpers _current_state, _transition_to_dict, _review_history_to_dict from get_workflow_info so the main loop reads top-to-bottom; drop the dead sharing-info comment; replace map()+closure with a list comprehension in get_children_info. Drop now-unused imports from __init__.py: bika.lims.api.snapshot, Products.ATContentTypes.utils.DT2dt, IFieldManager. * Add changelog entry for #95 * Add direct-call doctest for api.serialization Covers the extracted module: - Backward-compat identity of every re-exported name. - get_parent_info({}) short-circuit for the portal root. - get_workflow_info shape (workflow_info key, initial state, transitions). - get_workflow_info returning [] for objects with no assigned workflow. - Full get_info pipeline through /client/<uid> (url_info + parent_info). - ?complete=yes adding snapshot version. - ?complete=yes&workflow=yes adding workflow_info. * Add typed exception subclasses for the JSON API error envelope Introduce six typed subclasses of APIError so route code can raise a specific error class instead of calling api.fail(status, msg) with a magic number: 400 BadRequestError 401 UnauthorizedError 403 ForbiddenError 404 NotFoundError 409 ConflictError 422 ValidationError Each subclass carries its own default HTTP status; the previous APIError(status, message) positional signature becomes APIError(message, status=None) so typed subclasses can be raised as raise NotFoundError("...") without repeating the status number. Backward compatibility: - All typed errors inherit APIError. Any existing except APIError: handler catches every subclass. - api.fail(status, msg) still works and still raises APIError with the runtime status. Downstream callers do not have to change. - APIError.setStatus(x) alias retained for the same reason. Converts every api.fail() and raw APIError() call site inside senaite.jsonapi itself to the typed form: - request.get_request_data: BadRequestError (was APIError(400)) - v1/routes/content.get: NotFoundError for unknown resource - v1/routes/content.action: BadRequestError for unknown API member (was api.fail(500), which was misleading: the client asked for an unknown action, that is a 4xx, not a 5xx) - v1/routes/push: BadRequest/Unauthorized/NotFound as appropriate (was api.fail(500) for every failure mode, mixing client errors with server errors under one status) - v1/routes/users.login: UnauthorizedError (was api.fail(401)) - api.check_permission: Unauthorized/ForbiddenError Route-shape update for push.rst: the "non-registered adapter" case now returns 404 (correct: no consumer with that name is registered), where it previously returned 500. Depends on senaite/senaite.core#2998 for the JSON error envelope to actually surface the exception class name in the response body as a 'type' field. Without #2998, only the HTTP status changes are visible; the type field is discarded by the current handle_errors decorator. * Add changelog entry for #96 * Extract create/update/delete helpers to api/mutation Move the biggest remaining slice of api/__init__.py into a focused module: the three top-level route orchestrators (create_items, update_items, delete_items) plus their patch/put aliases, the low-level building blocks (create_object, update_object_with_data, deactivate_object, create_analysisrequest, find_target_container, validate_object) and the two permission checks (is_creation_allowed, is_update_allowed). All names remain importable from senaite.jsonapi.api via explicit re-exports at the bottom of __init__.py. v1/routes/content.action looks up 'api.create_items' / 'api.update_items' / 'api.delete_items' via getattr and continues to work unchanged. Semantic tightening while moving: - Every 'fail(401, ...)' inside the moved code becomes a proper ForbiddenError. HTTP 401 means 'log in and try again'; the fail sites (denied by permission gate, denied by adapter, container disallows type, ...) all mean 'you are authenticated but may not do this' -- which is 403. The two affected doctests (create.rst, update.rst) update their expected status from 401 to 403 to match. - 'fail(400, ...)' becomes BadRequestError, 'fail(404, ...)' becomes NotFoundError. Backward compat is preserved through the shared APIError base. Drop now-unused imports from __init__.py: copy, transaction, AccessControl.Unauthorized, create_ar, ICreate, IUpdate, IInfo, getAdapters, zope.deprecation.deprecate. * Add changelog entry for #97 * Accept REST verbs PUT/PATCH/DELETE on the action route The object-addressed action routes (/{resource}/{uid} and /{uid}) now register PUT, PATCH and DELETE alongside POST. When no explicit action segment is present in the URL, the action is derived from the HTTP method: PUT and PATCH map to 'update', DELETE maps to 'delete'. A probe against a running instance confirmed the @@API view already receives these verbs -- the view is an IPublishTraverse view that swallows the whole subpath and returns self, so at path exhaustion the published object is the view (which has __call__ but no PUT/PATCH attribute), and ZPublisher's WebDAV NullResource substitution does not apply. The verbs previously fell through to a werkzeug MethodNotAllowed because the route only registered POST; registering them is all that was needed. No WebDAV bypass, no plone.rest dependency, and the endpoints keep the single /@@API/senaite/v1/... URL shape. Resolution order in the action view: 1. Explicit action segment in the URL (unchanged). 2. HTTP verb (PUT/PATCH -> update, DELETE -> delete). 3. X-HTTP-Method-Override header (Backbone.js style, unchanged). A bare POST with none of these still errors, preserving the historic behaviour. Coverage: rest_verbs doctest drives real PUT/PATCH/DELETE through the browser's underlying WebTest app and asserts update/delete semantics, plus GET-unaffected and POST-without-action-still-errors regressions. * Add changelog entry for #98 * Add WebDAV verb bypass so PUT/PATCH/DELETE reliably reach the API The REST verb routing added in this PR relied on Zope not diverting PUT/PATCH/DELETE to WebDAV. But Zope flags every non-GET/POST request as maybe_webdav_client=1 by default (only XML-RPC clears it), and at path exhaustion may substitute a WebDAV NullResource. Whether the API view escapes that substitution depends on the view's acquisition shape and the Zope version -- so the verbs reaching the view was incidental, not guaranteed. Make it explicit and self-contained in senaite.jsonapi (independent of the plone.jsonapi.core version): an IPubStart subscriber clears maybe_webdav_client for API requests (path with an @@API / API segment) using PUT/PATCH/DELETE, before traversal consumes the flag. Genuine WebDAV requests to other content keep their flag, so real WebDAV is untouched. This works on the released plone.jsonapi.core 0.7.0 with no framework change. If a future plone.jsonapi.core ships an equivalent subscriber, clearing the flag twice is idempotent. Adds test_webdav covering the verb + path matrix and the segment-not-substring boundary. * Report the reason when object creation fails create_items caught each object's creation error, logged it, and then raised a generic "No Objects could be created", discarding the useful detail (for example the validation error {"field": "required field"}). Collect the per-object errors and include them in the raised message so callers see exactly what to fix instead of a generic failure. * Add changelog entry for #99 * Normalize UID references through the field manager, not the setter DexterityDataManager.set prioritized a content-type set<Name> mutator over the field manager. For a UID reference field (e.g. Department's manager) that bypassed UIDReferenceFieldMixin's normalization, so a unicode UID from a JSON payload was stored verbatim and later failed the field's ASCIILine value_type validation with WrongContainedType. Route UID reference fields through their field manager (which resolves objects/paths and coerces UIDs to native str). Every other field keeps prioritizing the setter, which also covers BBB properties that have no schema field (e.g. Department.DepartmentID). * Add changelog entry for #100 * Encode AT string field values to native str before validation AT field validators (isEmail, isDecimal, ...) expect a native str, but values from a JSON body arrive as unicode and fail with "expected 'string'" -- so EmailAddress, Price, VAT, DuplicateVariation could not be set through the JSON API. ATFieldManager._set now utf-8-encodes text values before validation; dicts/lists/other types are left untouched, so records/datagrid/reference fields are unaffected. Adds a create doctest that posts a JSON body with a unicode decimal. * Add changelog entry for #101 * Support DX Duration (Timedelta) fields via a field manager A DX DurationField is a zope.schema Timedelta, which JSON cannot carry, so fields like SamplePoint.sampling_frequency could not be set via the API: a raw {days, hours, minutes} mapping went through the set<Name> mutator and was later rejected as "wrong type". Add a DurationFieldManager that converts a {days, hours, minutes, seconds} mapping to/from a timedelta, register it for IDurationField, and route duration fields through the field manager in DexterityDataManager.set (as done for UID references). * Add changelog entry for #102 * Allow updating the setup configuration objects --------- Co-authored-by: Jordi Puiggené <jp@naralabs.com>
xispa
added a commit
that referenced
this pull request
Oct 6, 2026
commit 4044734 Merge: ba58d35 ae17335 Author: Jordi Puiggené <jp@naralabs.com> Date: Tue Oct 6 12:45:38 2026 +0200 Merge branch '2.x' of github.com:senaite/senaite.jsonapi into feature/allow-setup-config-update commit ba58d35 Author: Ramon Bartl <rb@ridingbytes.com> Date: Tue Oct 6 12:23:28 2026 +0200 Allow updating the Laboratory through the API (#112) * Fix Analyses field ignoring a plain list of analysis service UIDs * Assemble worksheets (layout + QC analyses) through the API * Skip generic re-validation on the IUpdate adapter path * Allow updating the Laboratory through the API Every update of the Laboratory was refused with a ForbiddenError, so the record could never be filled in through the API and stayed at its default title. That shows on every published report: an empty letterhead, and a footer reading "Laboratory . . . Phone : . Fax : . .". The cause is a side effect of the Laboratory's migration to Dexterity. is_update_allowed refuses children of the setup folder, which is right for the reference catalogs that live there, and the Laboratory moved under that folder with the migration. It is configuration, not a catalog, so it now counts as one of the setup singletons alongside bika_setup and senaite_setup. * Name the check for what it decides The helper answered "is this one of the setup singletons", and adding the laboratory to it made the name untrue: the laboratory is not a setup singleton, it is configuration that happens to live somewhere the parent check refuses. What the caller actually asks is whether the object holds the site's own configuration, so that is what it is called now. --------- Co-authored-by: Jordi Puiggené <jp@naralabs.com> commit 0e50a74 Author: Ramon Bartl <rb@ridingbytes.com> Date: Tue Oct 6 12:10:03 2026 +0200 Assemble worksheets (layout + QC analyses) through the API (#105) * Fix Analyses field ignoring a plain list of analysis service UIDs * Assemble worksheets (layout + QC analyses) through the API * Skip generic re-validation on the IUpdate adapter path --------- Co-authored-by: Jordi Puiggené <jp@naralabs.com> commit 5fbd012 Author: Ramon Bartl <rb@ridingbytes.com> Date: Tue Oct 6 12:04:17 2026 +0200 Fix Analyses field ignoring a plain list of analysis service UIDs (#104) commit fb8bf81 Merge: d0e7878 7b74f63 Author: Jordi Puiggené <jp@naralabs.com> Date: Mon Oct 5 14:54:02 2026 +0200 Merge branch '2.x' into feature/allow-setup-config-update commit d0e7878 Author: Ramon Bartl <rb@ridingbytes.com> Date: Mon Jul 27 15:08:58 2026 +0200 Allow updating the setup configuration objects commit ebd664d Author: Ramon Bartl <rb@ridingbytes.com> Date: Mon Jul 27 13:57:04 2026 +0200 Add changelog entry for #102 commit eb63d4e Author: Ramon Bartl <rb@ridingbytes.com> Date: Mon Jul 27 13:56:48 2026 +0200 Support DX Duration (Timedelta) fields via a field manager A DX DurationField is a zope.schema Timedelta, which JSON cannot carry, so fields like SamplePoint.sampling_frequency could not be set via the API: a raw {days, hours, minutes} mapping went through the set<Name> mutator and was later rejected as "wrong type". Add a DurationFieldManager that converts a {days, hours, minutes, seconds} mapping to/from a timedelta, register it for IDurationField, and route duration fields through the field manager in DexterityDataManager.set (as done for UID references). commit 22ca48d Author: Ramon Bartl <rb@ridingbytes.com> Date: Mon Jul 27 13:51:09 2026 +0200 Add changelog entry for #101 commit c8b04a2 Author: Ramon Bartl <rb@ridingbytes.com> Date: Mon Jul 27 13:50:47 2026 +0200 Encode AT string field values to native str before validation AT field validators (isEmail, isDecimal, ...) expect a native str, but values from a JSON body arrive as unicode and fail with "expected 'string'" -- so EmailAddress, Price, VAT, DuplicateVariation could not be set through the JSON API. ATFieldManager._set now utf-8-encodes text values before validation; dicts/lists/other types are left untouched, so records/datagrid/reference fields are unaffected. Adds a create doctest that posts a JSON body with a unicode decimal. commit 90ba4aa Author: Ramon Bartl <rb@ridingbytes.com> Date: Mon Jul 27 12:32:47 2026 +0200 Add changelog entry for #100 commit 26934bc Author: Ramon Bartl <rb@ridingbytes.com> Date: Mon Jul 27 12:32:47 2026 +0200 Normalize UID references through the field manager, not the setter DexterityDataManager.set prioritized a content-type set<Name> mutator over the field manager. For a UID reference field (e.g. Department's manager) that bypassed UIDReferenceFieldMixin's normalization, so a unicode UID from a JSON payload was stored verbatim and later failed the field's ASCIILine value_type validation with WrongContainedType. Route UID reference fields through their field manager (which resolves objects/paths and coerces UIDs to native str). Every other field keeps prioritizing the setter, which also covers BBB properties that have no schema field (e.g. Department.DepartmentID). commit 2c4860d Author: Ramon Bartl <rb@ridingbytes.com> Date: Mon Jul 27 11:55:01 2026 +0200 Add changelog entry for #99 commit 7ddaf3b Author: Ramon Bartl <rb@ridingbytes.com> Date: Mon Jul 27 11:54:26 2026 +0200 Report the reason when object creation fails create_items caught each object's creation error, logged it, and then raised a generic "No Objects could be created", discarding the useful detail (for example the validation error {"field": "required field"}). Collect the per-object errors and include them in the raised message so callers see exactly what to fix instead of a generic failure. commit 744b78f Author: Ramon Bartl <rb@ridingbytes.com> Date: Sat Jul 25 23:23:32 2026 +0200 Add WebDAV verb bypass so PUT/PATCH/DELETE reliably reach the API The REST verb routing added in this PR relied on Zope not diverting PUT/PATCH/DELETE to WebDAV. But Zope flags every non-GET/POST request as maybe_webdav_client=1 by default (only XML-RPC clears it), and at path exhaustion may substitute a WebDAV NullResource. Whether the API view escapes that substitution depends on the view's acquisition shape and the Zope version -- so the verbs reaching the view was incidental, not guaranteed. Make it explicit and self-contained in senaite.jsonapi (independent of the plone.jsonapi.core version): an IPubStart subscriber clears maybe_webdav_client for API requests (path with an @@API / API segment) using PUT/PATCH/DELETE, before traversal consumes the flag. Genuine WebDAV requests to other content keep their flag, so real WebDAV is untouched. This works on the released plone.jsonapi.core 0.7.0 with no framework change. If a future plone.jsonapi.core ships an equivalent subscriber, clearing the flag twice is idempotent. Adds test_webdav covering the verb + path matrix and the segment-not-substring boundary. commit 4dd0a8e Author: Ramon Bartl <rb@ridingbytes.com> Date: Sat Jul 25 22:46:31 2026 +0200 Add changelog entry for #98 commit e40f25b Author: Ramon Bartl <rb@ridingbytes.com> Date: Sat Jul 25 22:46:11 2026 +0200 Accept REST verbs PUT/PATCH/DELETE on the action route The object-addressed action routes (/{resource}/{uid} and /{uid}) now register PUT, PATCH and DELETE alongside POST. When no explicit action segment is present in the URL, the action is derived from the HTTP method: PUT and PATCH map to 'update', DELETE maps to 'delete'. A probe against a running instance confirmed the @@API view already receives these verbs -- the view is an IPublishTraverse view that swallows the whole subpath and returns self, so at path exhaustion the published object is the view (which has __call__ but no PUT/PATCH attribute), and ZPublisher's WebDAV NullResource substitution does not apply. The verbs previously fell through to a werkzeug MethodNotAllowed because the route only registered POST; registering them is all that was needed. No WebDAV bypass, no plone.rest dependency, and the endpoints keep the single /@@API/senaite/v1/... URL shape. Resolution order in the action view: 1. Explicit action segment in the URL (unchanged). 2. HTTP verb (PUT/PATCH -> update, DELETE -> delete). 3. X-HTTP-Method-Override header (Backbone.js style, unchanged). A bare POST with none of these still errors, preserving the historic behaviour. Coverage: rest_verbs doctest drives real PUT/PATCH/DELETE through the browser's underlying WebTest app and asserts update/delete semantics, plus GET-unaffected and POST-without-action-still-errors regressions. commit 4a53157 Author: Ramon Bartl <rb@ridingbytes.com> Date: Sat Jul 25 21:59:50 2026 +0200 Add changelog entry for #97 commit dccc934 Author: Ramon Bartl <rb@ridingbytes.com> Date: Sat Jul 25 21:55:58 2026 +0200 Extract create/update/delete helpers to api/mutation Move the biggest remaining slice of api/__init__.py into a focused module: the three top-level route orchestrators (create_items, update_items, delete_items) plus their patch/put aliases, the low-level building blocks (create_object, update_object_with_data, deactivate_object, create_analysisrequest, find_target_container, validate_object) and the two permission checks (is_creation_allowed, is_update_allowed). All names remain importable from senaite.jsonapi.api via explicit re-exports at the bottom of __init__.py. v1/routes/content.action looks up 'api.create_items' / 'api.update_items' / 'api.delete_items' via getattr and continues to work unchanged. Semantic tightening while moving: - Every 'fail(401, ...)' inside the moved code becomes a proper ForbiddenError. HTTP 401 means 'log in and try again'; the fail sites (denied by permission gate, denied by adapter, container disallows type, ...) all mean 'you are authenticated but may not do this' -- which is 403. The two affected doctests (create.rst, update.rst) update their expected status from 401 to 403 to match. - 'fail(400, ...)' becomes BadRequestError, 'fail(404, ...)' becomes NotFoundError. Backward compat is preserved through the shared APIError base. Drop now-unused imports from __init__.py: copy, transaction, AccessControl.Unauthorized, create_ar, ICreate, IUpdate, IInfo, getAdapters, zope.deprecation.deprecate. commit adfe046 Author: Ramon Bartl <rb@ridingbytes.com> Date: Sat Jul 25 12:52:29 2026 +0200 Add changelog entry for #96 commit ed52aac Author: Ramon Bartl <rb@ridingbytes.com> Date: Sat Jul 25 12:52:12 2026 +0200 Add typed exception subclasses for the JSON API error envelope Introduce six typed subclasses of APIError so route code can raise a specific error class instead of calling api.fail(status, msg) with a magic number: 400 BadRequestError 401 UnauthorizedError 403 ForbiddenError 404 NotFoundError 409 ConflictError 422 ValidationError Each subclass carries its own default HTTP status; the previous APIError(status, message) positional signature becomes APIError(message, status=None) so typed subclasses can be raised as raise NotFoundError("...") without repeating the status number. Backward compatibility: - All typed errors inherit APIError. Any existing except APIError: handler catches every subclass. - api.fail(status, msg) still works and still raises APIError with the runtime status. Downstream callers do not have to change. - APIError.setStatus(x) alias retained for the same reason. Converts every api.fail() and raw APIError() call site inside senaite.jsonapi itself to the typed form: - request.get_request_data: BadRequestError (was APIError(400)) - v1/routes/content.get: NotFoundError for unknown resource - v1/routes/content.action: BadRequestError for unknown API member (was api.fail(500), which was misleading: the client asked for an unknown action, that is a 4xx, not a 5xx) - v1/routes/push: BadRequest/Unauthorized/NotFound as appropriate (was api.fail(500) for every failure mode, mixing client errors with server errors under one status) - v1/routes/users.login: UnauthorizedError (was api.fail(401)) - api.check_permission: Unauthorized/ForbiddenError Route-shape update for push.rst: the "non-registered adapter" case now returns 404 (correct: no consumer with that name is registered), where it previously returned 500. Depends on senaite/senaite.core#2998 for the JSON error envelope to actually surface the exception class name in the response body as a 'type' field. Without #2998, only the HTTP status changes are visible; the type field is discarded by the current handle_errors decorator. commit e5aa41d Author: Ramon Bartl <rb@ridingbytes.com> Date: Sat Jul 25 12:11:47 2026 +0200 Add direct-call doctest for api.serialization Covers the extracted module: - Backward-compat identity of every re-exported name. - get_parent_info({}) short-circuit for the portal root. - get_workflow_info shape (workflow_info key, initial state, transitions). - get_workflow_info returning [] for objects with no assigned workflow. - Full get_info pipeline through /client/<uid> (url_info + parent_info). - ?complete=yes adding snapshot version. - ?complete=yes&workflow=yes adding workflow_info. commit 5298bfb Author: Ramon Bartl <rb@ridingbytes.com> Date: Sat Jul 25 12:05:57 2026 +0200 Add changelog entry for #95 commit 908f2dc Author: Ramon Bartl <rb@ridingbytes.com> Date: Sat Jul 25 12:05:47 2026 +0200 Extract serialization helpers to api/serialization Move the six JSON-representation helpers out of the god-module api/__init__.py into a focused api/serialization.py: - get_info (main entry point: brain/object -> JSON-ready dict) - get_url_info (uid, url, api_url) - get_parent_info (parent_id, parent_uid, parent_url) - get_children_info (folderish contents) - get_file_info (file field payload) - get_workflow_info (assigned workflows + current state + transitions) All six names remain importable from senaite.jsonapi.api via explicit re-exports at the bottom of __init__.py, so fieldmanagers.py (which calls api.get_file_info and api.get_url_info) and any downstream users keep working unchanged. Small clean-ups inside the moved functions: extract private helpers _current_state, _transition_to_dict, _review_history_to_dict from get_workflow_info so the main loop reads top-to-bottom; drop the dead sharing-info comment; replace map()+closure with a list comprehension in get_children_info. Drop now-unused imports from __init__.py: bika.lims.api.snapshot, Products.ATContentTypes.utils.DT2dt, IFieldManager. commit 3483e73 Author: Ramon Bartl <rb@ridingbytes.com> Date: Sat Jul 25 11:38:55 2026 +0200 Use cookie-login browser for /settings positive path After stacking on PR-B, the /settings route requires the Manage portal permission. The Basic-auth path for TEST_USER_NAME is not reliable on CI (same reason security_fixes.rst switched away from it), so use self.getBrowser() which does form login and is known to work. commit 300237a Author: Ramon Bartl <rb@ridingbytes.com> Date: Sat Jul 25 11:23:07 2026 +0200 Add real-value assertions to api.settings doctest Set concrete IMailSchema fields (smtp_host, smtp_port, email_from_name, email_from_address) so the extracted helpers can be verified round-trip against known values instead of just shape. commit 82a0d2b Author: Ramon Bartl <rb@ridingbytes.com> Date: Sat Jul 25 11:37:49 2026 +0200 Drop now-unused getAdapter import CI lint flagged zope.component.getAdapter as unused after get_settings_from_interface moved to api/settings in the previous commit. Keep ploneapi (still used by check_permission). commit bd22f32 Author: Ramon Bartl <rb@ridingbytes.com> Date: Sat Jul 25 11:14:24 2026 +0200 Add direct-call doctest for api.settings Covers the extracted module without relying on the /settings and /registry routes (which currently trip over non-JSON-serializable registry values under a Manager account). Exercises: - Backward-compat identity of every re-exported name. - CONTROLPANEL_INTERFACE_MAPPING keys. - get_settings_from_interface shape + JSON-serializability filter. - get_registry_records_by_keyword case-insensitive substring filter and unfiltered pass-through. - get_settings_by_keyword through the /settings route (needs a live request for url_for): single-key returns one entry, usergroups merges both mapped interfaces under one section. commit c2e23b9 Author: Ramon Bartl <rb@ridingbytes.com> Date: Sat Jul 25 10:53:00 2026 +0200 Add changelog entry for #94 commit e2fd102 Author: Ramon Bartl <rb@ridingbytes.com> Date: Sat Jul 25 10:52:43 2026 +0200 Extract registry and settings helpers to api/settings Move get_registry_records_by_keyword, get_settings_by_keyword, get_settings_from_interface, and the CONTROLPANEL_INTERFACE_MAPPING constant out of api/__init__.py into a focused api/settings.py. The four names remain importable from senaite.jsonapi.api via explicit re-exports at the bottom of __init__.py, so route code and any downstream users keep working unchanged. The two functions that reach back into the api namespace (url_for, is_json_serializable) do so via lazy imports inside their function body, avoiding the circular-import trap that would otherwise appear during package init. Also drop the six now-unused control-panel schema imports and the zope.schema getFieldNames import from __init__.py. commit 47cf768 Author: Ramon Bartl <rb@ridingbytes.com> Date: Sat Jul 25 10:32:37 2026 +0200 Add changelog entry for #93 commit acecdd7 Author: Ramon Bartl <rb@ridingbytes.com> Date: Sat Jul 25 10:32:27 2026 +0200 Extract user helpers to senaite.jsonapi.api.users Move is_anonymous, get_current_user, get_member_ids, get_user, and get_user_properties out of the god-module api/__init__.py into a focused api/users.py. The five names remain importable from senaite.jsonapi.api via explicit re-exports at the bottom of __init__.py, so downstream code (senaite.core, add-ons, docs, tests) needs no change. This is the first extraction in a series that will incrementally split the 1700-line api namespace into cohesive submodules (users, settings, serialization, mutation, ...) without touching the public import surface. commit 564f533 Author: Ramon Bartl <rb@ridingbytes.com> Date: Sat Jul 25 10:30:26 2026 +0200 Convert api module to package for future extractions Pure rename: src/senaite/jsonapi/api.py -> src/senaite/jsonapi/api/__init__.py. Python treats a package (directory with __init__.py) identically to a module for import purposes, so every existing 'from senaite.jsonapi import api' and 'from senaite.jsonapi.api import X' keeps working without change. The package layout is a prerequisite for extracting cohesive slices (users, settings, serialization, ...) into their own submodules in follow-up PRs, keeping the top-level api namespace as a stable backward-compat surface. commit 1385d5f Author: Ramon Bartl <rb@ridingbytes.com> Date: Sat Jul 25 11:28:09 2026 +0200 Drop redundant Manager positive-path assertion in security_fixes The Manager-can-enumerate path is already exercised by users.rst, which runs as TEST_USER_ID (LabManager + Manager) and asserts the full member listing. Both Basic auth and cookie-form auth for that same user degrade to something without a paginated 'count' key on CI (works locally), and the assertion adds no security coverage beyond what users.rst already provides. Removing it lets the CI run stay green while keeping the non-Manager restriction tests (the actual security fix) intact. commit ad149e0 Author: Ramon Bartl <rb@ridingbytes.com> Date: Sat Jul 25 11:24:07 2026 +0200 Use fixture cookie-login for Manager path in security_fixes doctest Basic auth with TEST_USER_NAME/TEST_USER_PASSWORD passes locally but fails on CI (KeyError on 'count' at line 91) because the response falls back to an error shape when the credentials do not authenticate. Switch that single assertion to self.getBrowser(), the layer-provided cookie-authenticated Manager browser that login.rst already uses successfully across every CI build. Non-manager Basic-auth paths (test_labclerk_0, etc.) keep using the as_user helper because base.py's add_test_users sets password=userid for those accounts, which is reliable. commit 9712c17 Merge: 27c4038 21c7493 Author: Ramon Bartl <rb@ridingbytes.com> Date: Sat Jul 25 10:05:39 2026 +0200 Merge remote-tracking branch 'origin/2.x' into security/pr-b # Conflicts: # docs/changelog.rst commit 27c4038 Author: Ramon Bartl <rb@ridingbytes.com> Date: Sat Jul 25 09:45:18 2026 +0200 Add changelog entry for #92 commit b3b514a Author: Ramon Bartl <rb@ridingbytes.com> Date: Sat Jul 25 09:45:17 2026 +0200 Log deprecation warning on GET /login with credentials Credentials submitted via GET land in access logs, Referer headers, and browser history. Warn on this now and plan removal for 2.8.0. GET without credentials (basic-auth handoff) is unaffected. commit 67930d5 Author: Ramon Bartl <rb@ridingbytes.com> Date: Sat Jul 25 09:45:06 2026 +0200 Restrict /users listing to managers to prevent enumeration Any authenticated user could list every account and inspect any single user by id. Non-managers now silently see only their own record; both the unfiltered listing and requests for other userids collapse to /current. Managers retain full listing access. Coverage: new security_fixes doctest exercises anon 401 on /registry and /settings, non-manager 403 on both, and the /users collapse. commit 53220bf Author: Ramon Bartl <rb@ridingbytes.com> Date: Sat Jul 25 09:44:43 2026 +0200 Require Manage portal permission for /registry and /settings Both routes returned control-panel data to anonymous callers. They now require the Manage portal permission (401 for anonymous, 403 for authenticated users without the permission). A new api.check_permission helper centralizes the check.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
create_itemsnow includes the underlying error(s) in its response when nothing could be created, instead of the generic "No Objects could be created".Why
When an object failed to create,
create_itemscaught the exception, logged it, and raised a genericBadRequestError("No Objects could be created")— discarding the actual reason. For example, creating anAnalysisCategorywithout its requireddepartmentfield produced only:while the real cause was right there in the logs:
Callers (including agents driving the API) had to dig through server logs to find out which required fields were missing.
Change
create_itemscollects the per-object error messages and surfaces them in the raised error:The authoritative validation detail comes straight from SENAITE, so there is no duplicated list of required fields to maintain. Per-object savepoint rollback and the partial-success path (return whatever succeeded) are unchanged.
Stacking
Stacked on top of #98 (
feature/rest-verbs).