Skip to content

Commit ef4b6d7

Browse files
authored
Enforce the AccessJSONAPI permission on state-changing JSON API routes (#2919)
* Enforce the AccessJSONAPI permission on state-changing JSON API routes The state-changing JSON API routes (update, update_many, remove, doActionFor, doActionFor_many) and the user-enumeration route (getusers) did not enforce the 'senaite.core: Access JSON API' permission, unlike the create route. The @@API view is published with zope2.View, which Anonymous holds on the site root, so these routes were reachable by anonymous and under-privileged callers (CWE-862 Missing Authorization). Add a shared check_jsonapi_permission() helper in bika.lims.jsonapi and call it on the resolved target object before any mutation or state change in every state-changing route. getusers is checked against the site root before enumerating members. The create route is refactored to use the same helper, so the permission check now has a single source of truth. The check is placed outside the routes' try/except blocks so the Unauthorized is not masked as a BadRequest. Add a regression test (test_jsonapi_security) covering all gated routes for anonymous and under-privileged (Member) callers, asserting the call is rejected, plus a check that a privileged caller still passes the gate and that the shared helper behaves correctly. * Authenticate jsonapi security tests via Basic Auth The privileged/member browser tests rendered the Plone login form, which under the test layer triggers an unrelated z3c.form widget template error ('TextWidget' object has no attribute 'attrs' in datetimewidget_input.pt) and fails in CI. Authenticate through an HTTP Basic Auth header instead, which the JSON API tests do not need a rendered form for, keeping the tests focused on the permission check.
1 parent 5d1bb21 commit ef4b6d7

8 files changed

Lines changed: 258 additions & 30 deletions

File tree

CHANGES.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ Changelog
44
2.7.0 (unreleased)
55
------------------
66

7+
- #2919 Enforce the AccessJSONAPI permission on state-changing JSON API routes
78
- #2918 Fix Lab Information setup data import after Laboratory migration to Dexterity
89
- #2917 Fix `ClientID` is not displayed in samples listing, but client name
910
- #2916 Fix msgid collision on `description_calculation_imports` in Calculation content type

src/bika/lims/jsonapi/__init__.py

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,20 +18,37 @@
1818
# Copyright 2018-2025 by it's authors.
1919
# Some rights reserved, see README and LICENSE.
2020

21+
import json
22+
import sys
23+
import traceback
24+
25+
import Missing
26+
import six
27+
from AccessControl import Unauthorized
28+
from AccessControl import getSecurityManager
29+
from bika.lims import api
30+
from bika.lims import logger
31+
from bika.lims.utils import to_utf8
2132
from plone.app.textfield import RichTextValue
2233
from Products.Archetypes.config import TOOL_NAME
2334
from Products.CMFCore.utils import getToolByName
2435
from senaite.core.browser.fields.parsing import parse_record_literal
36+
from senaite.core.permissions import AccessJSONAPI
2537

26-
from bika.lims import api
27-
from bika.lims.utils import to_utf8
28-
from bika.lims import logger
2938

30-
import json
31-
import Missing
32-
import six
33-
import sys
34-
import traceback
39+
def check_jsonapi_permission(obj):
40+
"""Check the AccessJSONAPI permission on the given object
41+
42+
Raises Unauthorized if the current user does not hold the
43+
`senaite.core: Access JSON API` permission on the passed-in object.
44+
This guards the state-changing JSON API routes against anonymous and
45+
under-privileged callers (CWE-862).
46+
"""
47+
if getSecurityManager().checkPermission(AccessJSONAPI, obj):
48+
return
49+
msg = "You don't have the '{0}' permission on {1}".format(
50+
AccessJSONAPI, obj.absolute_url())
51+
raise Unauthorized(msg)
3552

3653

3754
def handle_errors(f):

src/bika/lims/jsonapi/create.py

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,14 @@
1919
# Some rights reserved, see README and LICENSE.
2020

2121
import transaction
22-
from AccessControl import Unauthorized
23-
from AccessControl import getSecurityManager
24-
from senaite.core.idserver import renameAfterCreation
22+
from bika.lims.jsonapi import check_jsonapi_permission
2523
from bika.lims.jsonapi import set_fields_from_request
2624
from bika.lims.utils import tmpID
2725
from plone.jsonapi.core import router
2826
from plone.jsonapi.core.interfaces import IRouteProvider
2927
from Products.Archetypes.event import ObjectInitializedEvent
3028
from Products.CMFPlone.utils import _createObjectByType
31-
from senaite.core.permissions import AccessJSONAPI
29+
from senaite.core.idserver import renameAfterCreation
3230
from zExceptions import BadRequest
3331
from zope import event
3432
from zope import interface
@@ -176,10 +174,7 @@ def create(self, context, request):
176174
site_path = request['PATH_INFO'].replace("/@@API/create", "")
177175
parent = context.restrictedTraverse(str(site_path + obj_path))
178176
# normal permissions still apply for this user
179-
if not getSecurityManager().checkPermission(AccessJSONAPI, parent):
180-
msg = "You don't have the '{0}' permission on {1}".format(
181-
AccessJSONAPI, parent.absolute_url())
182-
raise Unauthorized(msg)
177+
check_jsonapi_permission(parent)
183178

184179
obj_id = request.get("obj_id", "")
185180
_renameAfterCreation = False

src/bika/lims/jsonapi/doactionfor.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,16 @@
1818
# Copyright 2018-2025 by it's authors.
1919
# Some rights reserved, see README and LICENSE.
2020

21+
import json
22+
23+
import transaction
24+
from bika.lims.jsonapi import check_jsonapi_permission
2125
from bika.lims.jsonapi.read import read
2226
from plone.jsonapi.core import router
2327
from plone.jsonapi.core.interfaces import IRouteProvider
2428
from Products.CMFCore.utils import getToolByName
2529
from zExceptions import BadRequest
2630
from zope import interface
27-
import json
28-
29-
import transaction
3031

3132

3233
class doActionFor(object):
@@ -73,8 +74,10 @@ def do_action_for(self, context, request):
7374
if len(objects) == 0:
7475
raise BadRequest("No matching objects found")
7576
for obj_dict in objects:
77+
obj = uc(UID=obj_dict['UID'])[0].getObject()
78+
# normal permissions still apply for this user
79+
check_jsonapi_permission(obj)
7680
try:
77-
obj = uc(UID=obj_dict['UID'])[0].getObject()
7881
workflow.doActionFor(obj, action)
7982
obj.reindexObject()
8083
except Exception as e:
@@ -114,6 +117,8 @@ def do_action_for_many(self, context, request):
114117
if not obj_path.startswith("/"):
115118
obj_path = "/" + obj_path
116119
obj = context.restrictedTraverse(str(site_path + obj_path))
120+
# normal permissions still apply for this user
121+
check_jsonapi_permission(obj)
117122
if obj_path.startswith(site_path):
118123
obj_path = obj_path[len(site_path):]
119124
try:

src/bika/lims/jsonapi/getusers.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
# Copyright 2018-2025 by it's authors.
1919
# Some rights reserved, see README and LICENSE.
2020

21+
from bika.lims.jsonapi import check_jsonapi_permission
2122
from plone.jsonapi.core import router
2223
from plone.jsonapi.core.interfaces import IRouteProvider
2324
from Products.CMFCore.utils import getToolByName
@@ -67,12 +68,15 @@ def getusers(self, context, request):
6768
>>> browser.contents
6869
'No roles specified'
6970
"""
71+
# normal permissions still apply for this user
72+
check_jsonapi_permission(context)
73+
7074
roles = request.get('roles','')
7175

7276
if len(roles) == 0:
7377
raise BadRequest("No roles specified")
74-
75-
mtool = getToolByName(context, 'portal_membership')
78+
79+
mtool = getToolByName(context, 'portal_membership')
7680
users = []
7781
for user in mtool.searchForMembers(roles=roles):
7882
uid = user.getId()

src/bika/lims/jsonapi/remove.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,13 @@
1818
# Copyright 2018-2025 by it's authors.
1919
# Some rights reserved, see README and LICENSE.
2020

21+
import transaction
22+
from bika.lims.jsonapi import check_jsonapi_permission
2123
from plone.jsonapi.core import router
2224
from plone.jsonapi.core.interfaces import IRouteProvider
2325
from Products.CMFCore.utils import getToolByName
2426
from zExceptions import BadRequest
2527
from zope import interface
26-
import transaction
2728

2829

2930
class Remove(object):
@@ -78,14 +79,17 @@ def remove(self, context, request):
7879
"success": True,
7980
"error": False,
8081
}
81-
82+
8283
data = uc(UID=_uid)
8384
if not data:
8485
raise BadRequest("No objects found")
85-
86+
8687
for proxy in data:
88+
obj = proxy.getObject()
89+
# normal permissions still apply for this user
90+
check_jsonapi_permission(obj)
8791
try:
88-
parent = proxy.getObject().aq_parent
92+
parent = obj.aq_parent
8993
parent.manage_delObjects([proxy.id])
9094
except Exception as e:
9195
savepoint.rollback()

src/bika/lims/jsonapi/update.py

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,17 @@
1818
# Copyright 2018-2025 by it's authors.
1919
# Some rights reserved, see README and LICENSE.
2020

21+
import json
22+
23+
import six
24+
import transaction
25+
from bika.lims.jsonapi import check_jsonapi_permission
2126
from bika.lims.jsonapi import set_fields_from_request
22-
from Products.CMFCore.utils import getToolByName
2327
from plone.jsonapi.core import router
2428
from plone.jsonapi.core.interfaces import IRouteProvider
29+
from Products.CMFCore.utils import getToolByName
2530
from zExceptions import BadRequest
2631
from zope import interface
27-
import json
28-
import six
29-
import transaction
3032

3133

3234
class Update(object):
@@ -146,6 +148,9 @@ def update(self, context, request):
146148
ret['error'] = True
147149
return ret
148150

151+
# normal permissions still apply for this user
152+
check_jsonapi_permission(obj)
153+
149154
try:
150155
fields = set_fields_from_request(obj, request)
151156
if not fields:
@@ -225,6 +230,8 @@ def update_many(self, context, request):
225230
if obj_path.startswith(site_path):
226231
obj_path = obj_path[len(site_path):]
227232
obj = context.restrictedTraverse(str(site_path + obj_path))
233+
# normal permissions still apply for this user
234+
check_jsonapi_permission(obj)
228235
this_ret = {
229236
"url": router.url_for("update_many", force_external=True),
230237
"success": False,

0 commit comments

Comments
 (0)