GH-581: Check object permissions before calling the Objecten API - #895
Conversation
The four read methods on ObjectenApiClient performed the outbound HTTP call before requirePermission, so a 404/4xx from the Objecten API propagated out of .retrieve().body() ahead of the permission check. An unauthorized caller could therefore distinguish "object exists" from "object does not exist", and denied requests still made a pointless outbound call. Move the requirePermission block above buildRestClient in getObject, getObjectRecord, getObjectsByObjecttypeUrl and getObjectsByObjecttypeUrlWithSearchParams. Nothing is lost by checking first: the entity passed is a fieldless Object(), so the check has no dependency on the response. The four write methods already checked first and are unchanged. ObjectenApiClientTest now asserts, for each of the four read methods, that no request reaches the mock API when permission is denied.
|
Warning Review limit reached
Next review available in: 118 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…cten-api-check-permission-before-fetch
Nightly maintenance run. - Moved the 'Object permissions are checked before the object is retrieved' bugfix note from 13.41.0 to 13.42.0, which is the current release-notes folder on next-minor.
…cten-api-check-permission-before-fetch # Conflicts: # documentation/release-notes/13.x.x/13.42.0/README.md
2 participants
https://github.com/generiekzaakafhandelcomponent/atlas-internal/issues/581
A user without permission to view objects was refused only after the object had already been
retrieved from the Objecten API. Because the API's answer came back first, the error such a user
received revealed whether the object existed — a one-bit existence oracle — and every request
that was going to be denied still made a pointless outbound call. The permission check now runs
before anything is requested, in all four read operations. The write operations already checked
first and are unchanged.
Nothing is lost by checking first: the entity passed to the check is a fieldless
Object(), sothe check has no dependency on the response.
The four new tests each fail against the unpatched code with
HttpClientErrorException$NotFound— that failure is the leak, which is why the mock enqueues a404 rather than a success.
Scope
This fixes the concrete defect in issue 581 only. The issue also raises per-object scoping,
which is a permission-model question rather than a bug:
Objectis deliberately a fieldlesscapability marker, so no permission condition can scope it. That is being handled as a roadmap
item and the issue should not be closed as fixed on the back of this PR — a
viewgrant stillmeans "may read any object in a configured Objecten API".
Worth noting for the reporter: the write paths cited in the issue (
createObject,objectPatch,objectUpdate,deleteObject) already checked permission before the outbound call, so thewrite/delete escalation described there does not apply to this defect.
Not verified against a running application: this is a reordering inside the API client with no UI
surface of its own, covered by unit tests that assert no request reaches the mock API when
permission is denied.
Note for the reviewer: this and #580 both add to
documentation/release-notes/13.x.x/13.41.0/README.mdand will conflict there on merge. The conflict is additive — keep both entries.