Repository navigation
Allow updating the setup configuration objects - #103
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.
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).
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.
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).
xispa
approved these changes
Oct 6, 2026
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
2 participants
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.
Description
is_update_allowed()refuses any object whose parent is the portal root. That denylist also catches the setup configuration singletons — the ATbika_setupand the DXsenaite_setup— which sit directly under the portal. As a result, site-wide settings stored on those objects (for exampleSelfVerificationEnabled) cannot be changed through the JSON API at all, even by a user who holds the required permission.This PR adds an
is_setup()helper and treats the two setup singletons as an explicit exception: the objects themselves become updatable, while their children stay read-only (the existing parent-based checks still apply to everything else). Authorization is unchanged — theModifyPortalContentpermission check in the data manager still gates every write — so only privileged users can update the setup config.Stacked on #102.
Current behavior before PR
Updating the setup config object returns
Update of /<site>/bika_setup is not allowed(likewise for the DXsenaite_setup), so settings such as self-verification cannot be toggled via the API.Desired behavior after PR is merged
The setup configuration singletons can be updated through the API (subject to the usual permission check), while objects contained in the setup folders remain read-only.