From b3515c52d79f385760e8c9c26451748ec0066eb4 Mon Sep 17 00:00:00 2001 From: Klaas Schuijtemaker Date: Tue, 11 Aug 2026 15:18:01 +0200 Subject: [PATCH 1/2] fix: check object permissions before calling the Objecten API 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. --- .../objectenapi/client/ObjectenApiClient.kt | 60 ++++++------- .../client/ObjectenApiClientTest.kt | 84 +++++++++++++++++++ .../release-notes/13.x.x/13.41.0/README.md | 6 ++ 3 files changed, 120 insertions(+), 30 deletions(-) diff --git a/backend/zgw/objecten-api/src/main/kotlin/com/ritense/objectenapi/client/ObjectenApiClient.kt b/backend/zgw/objecten-api/src/main/kotlin/com/ritense/objectenapi/client/ObjectenApiClient.kt index 5c38a658ac..b0cb4ad6d5 100644 --- a/backend/zgw/objecten-api/src/main/kotlin/com/ritense/objectenapi/client/ObjectenApiClient.kt +++ b/backend/zgw/objecten-api/src/main/kotlin/com/ritense/objectenapi/client/ObjectenApiClient.kt @@ -51,12 +51,6 @@ class ObjectenApiClient( authentication: ObjectenApiAuthentication, objectUrl: URI ): ObjectWrapper { - val result = buildRestClient(authentication) - .get() - .uri(objectUrl) - .retrieve() - .body()!! - authorizationService.requirePermission( EntityAuthorizationRequest( Object::class.java, @@ -65,6 +59,12 @@ class ObjectenApiClient( ) ) + val result = buildRestClient(authentication) + .get() + .uri(objectUrl) + .retrieve() + .body()!! + outboxService.send { ObjectViewed( result.url.toString(), @@ -79,6 +79,14 @@ class ObjectenApiClient( objectUrl: URI, index: Int ): ObjectRecord { + authorizationService.requirePermission( + EntityAuthorizationRequest( + Object::class.java, + ObjectActionProvider.VIEW, + Object() + ) + ) + val recordUrl = UriComponentsBuilder .fromUri(objectUrl) .pathSegment(index.toString()) @@ -91,14 +99,6 @@ class ObjectenApiClient( .retrieve() .body()!! - authorizationService.requirePermission( - EntityAuthorizationRequest( - Object::class.java, - ObjectActionProvider.VIEW, - Object() - ) - ) - outboxService.send { ObjectViewed( objectUrl.toString(), @@ -116,6 +116,14 @@ class ObjectenApiClient( ordering: String? = null, pageable: Pageable ): ObjectsList { + authorizationService.requirePermission( + EntityAuthorizationRequest( + Object::class.java, + ObjectActionProvider.VIEW_LIST, + Object() + ) + ) + val objectTypeUrl = UriComponentsBuilder.newInstance() .uri(objecttypesApiUrl) .host(objecttypesApiUrl.host) @@ -156,14 +164,6 @@ class ObjectenApiClient( .retrieve() .body()!! - authorizationService.requirePermission( - EntityAuthorizationRequest( - Object::class.java, - ObjectActionProvider.VIEW_LIST, - Object() - ) - ) - outboxService.send { ObjectsListed( objectMapper.valueToTree(result.results) @@ -181,6 +181,14 @@ class ObjectenApiClient( ordering: String? = null, pageable: Pageable ): ObjectsList { + authorizationService.requirePermission( + EntityAuthorizationRequest( + Object::class.java, + ObjectActionProvider.VIEW_LIST, + Object() + ) + ) + val objectTypeUrl = UriComponentsBuilder.newInstance() .uri(objecttypesApiUrl) .host(objecttypesApiUrl.host) @@ -222,14 +230,6 @@ class ObjectenApiClient( .retrieve() .body()!! - authorizationService.requirePermission( - EntityAuthorizationRequest( - Object::class.java, - ObjectActionProvider.VIEW_LIST, - Object() - ) - ) - outboxService.send { ObjectsListed( objectMapper.valueToTree(result.results) diff --git a/backend/zgw/objecten-api/src/test/kotlin/com/ritense/objectenapi/client/ObjectenApiClientTest.kt b/backend/zgw/objecten-api/src/test/kotlin/com/ritense/objectenapi/client/ObjectenApiClientTest.kt index 1ebadd6eec..6ac5d2f4c1 100644 --- a/backend/zgw/objecten-api/src/test/kotlin/com/ritense/objectenapi/client/ObjectenApiClientTest.kt +++ b/backend/zgw/objecten-api/src/test/kotlin/com/ritense/objectenapi/client/ObjectenApiClientTest.kt @@ -46,7 +46,9 @@ import org.junit.jupiter.api.Test import org.junit.jupiter.api.TestInstance import org.junit.jupiter.api.assertThrows import org.mockito.Mockito +import org.mockito.kotlin.any import org.mockito.kotlin.argumentCaptor +import org.mockito.kotlin.doThrow import org.mockito.kotlin.mock import org.mockito.kotlin.reset import org.mockito.kotlin.times @@ -55,6 +57,7 @@ import org.skyscreamer.jsonassert.JSONAssert import org.springframework.data.domain.PageRequest import org.springframework.http.HttpHeaders.CONTENT_TYPE import org.springframework.http.MediaType.APPLICATION_JSON_VALUE +import org.springframework.security.access.AccessDeniedException import org.springframework.web.client.HttpClientErrorException import org.springframework.web.client.RestClient import org.springframework.web.reactive.function.client.ClientRequest @@ -305,6 +308,27 @@ internal class ObjectenApiClientTest { verify(outboxService, times(0)).send(eventCapture.capture()) } + @Test + fun `should throw exception and not call api when not authorized to get object`() { + assertReadDeniedWithoutCallingApi { client, deniedApi -> + client.getObject( + TestAuthentication(), + deniedApi.url("/some-object").toUri() + ) + } + } + + @Test + fun `should throw exception and not call api when not authorized to get objectrecord`() { + assertReadDeniedWithoutCallingApi { client, deniedApi -> + client.getObjectRecord( + TestAuthentication(), + deniedApi.url("/some-object").toUri(), + 2 + ) + } + } + @Test fun `should get objectslist`() { val client = ObjectenApiClient(restClientBuilder, outboxService, objectMapper, authorizationService) @@ -472,6 +496,20 @@ internal class ObjectenApiClientTest { verify(outboxService, times(0)).send(eventCapture.capture()) } + @Test + fun `should throw exception and not call api when not authorized to get objects by object type url`() { + assertReadDeniedWithoutCallingApi { client, deniedApi -> + client.getObjectsByObjecttypeUrl( + TestAuthentication(), + deniedApi.url("/some-object").toUri(), + deniedApi.url("/some-objectTypesApi").toUri(), + "typeId", + "", + PageRequest.of(0, 10) + ) + } + } + @Test fun `should send outbox message when getting objects by object type url with search params`() { val client = ObjectenApiClient(restClientBuilder, outboxService, objectMapper, authorizationService) @@ -565,6 +603,21 @@ internal class ObjectenApiClientTest { verify(outboxService, times(0)).send(eventCapture.capture()) } + @Test + fun `should throw exception and not call api when not authorized to get objects by object type url with search params`() { + assertReadDeniedWithoutCallingApi { client, deniedApi -> + client.getObjectsByObjecttypeUrlWithSearchParams( + authentication = TestAuthentication(), + objecttypesApiUrl = deniedApi.url("/some-object").toUri(), + objectsApiUrl = deniedApi.url("/some-objectTypesApi").toUri(), + objectypeId = "typeId", + searchString = "test", + ordering = "ordering", + pageable = PageRequest.of(0, 10) + ) + } + } + @Test fun `should send outbox message on creating object`() { val client = ObjectenApiClient(restClientBuilder, outboxService, objectMapper, authorizationService) @@ -1118,6 +1171,37 @@ internal class ObjectenApiClientTest { .setBody(body) } + /** + * Asserts that a read operation is denied before any request leaves for the Objecten API. + * + * The permission check has to happen before the fetch: otherwise the response status of the + * Objecten API surfaces to an unauthorized caller, who can then tell an existing object apart + * from a non-existent one. The mock API answers 404 to make that leak visible - when the check + * runs too late, the caller sees the 404 instead of an [AccessDeniedException]. + * + * A dedicated [MockWebServer] is used so that the enqueued response cannot leak into the + * queue of another test in this class. + */ + private fun assertReadDeniedWithoutCallingApi(invoke: (ObjectenApiClient, MockWebServer) -> Unit) { + val deniedApi = MockWebServer() + deniedApi.start() + try { + deniedApi.enqueue(mockResponse("").setResponseCode(404)) + + val deniedAuthorizationService: AuthorizationService = mock { + on { this.requirePermission(any()) } doThrow AccessDeniedException("Unauthorized") + } + val client = ObjectenApiClient(restClientBuilder, outboxService, objectMapper, deniedAuthorizationService) + + assertThrows { invoke(client, deniedApi) } + + assertEquals(0, deniedApi.requestCount) + verify(outboxService, times(0)).send(any()) + } finally { + deniedApi.shutdown() + } + } + class TestAuthentication : ObjectenApiAuthentication { override fun applyAuth(builder: RestClient.Builder): RestClient.Builder { return builder.defaultHeaders { headers -> diff --git a/documentation/release-notes/13.x.x/13.41.0/README.md b/documentation/release-notes/13.x.x/13.41.0/README.md index 1e2979eef9..0dad95cac0 100644 --- a/documentation/release-notes/13.x.x/13.41.0/README.md +++ b/documentation/release-notes/13.x.x/13.41.0/README.md @@ -74,3 +74,9 @@ * **Exporting case definitions** Case definitions that had a process definition removed via the database were unable to be exported. + +* **Object permissions are checked before the object is retrieved** + + A user without permission to view objects is now refused before anything is requested from the Objecten API. + Previously the object was retrieved first, so the answer of the Objecten API could tell such a user whether an + object exists. From 7bfa8c5e03dd94a239658f707f4676a06b7ef11f Mon Sep 17 00:00:00 2001 From: Klaas Schuijtemaker Date: Wed, 12 Aug 2026 14:52:38 +0200 Subject: [PATCH 2/2] docs: move release note to 13.42.0 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. --- documentation/release-notes/13.x.x/13.41.0/README.md | 6 ------ documentation/release-notes/13.x.x/13.42.0/README.md | 6 +++++- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/documentation/release-notes/13.x.x/13.41.0/README.md b/documentation/release-notes/13.x.x/13.41.0/README.md index 0dad95cac0..1e2979eef9 100644 --- a/documentation/release-notes/13.x.x/13.41.0/README.md +++ b/documentation/release-notes/13.x.x/13.41.0/README.md @@ -74,9 +74,3 @@ * **Exporting case definitions** Case definitions that had a process definition removed via the database were unable to be exported. - -* **Object permissions are checked before the object is retrieved** - - A user without permission to view objects is now refused before anything is requested from the Objecten API. - Previously the object was retrieved first, so the answer of the Objecten API could tell such a user whether an - object exists. diff --git a/documentation/release-notes/13.x.x/13.42.0/README.md b/documentation/release-notes/13.x.x/13.42.0/README.md index 5691046043..8e68f1da19 100644 --- a/documentation/release-notes/13.x.x/13.42.0/README.md +++ b/documentation/release-notes/13.x.x/13.42.0/README.md @@ -18,4 +18,8 @@ ## Bugfixes -* New bugfix. +* **Object permissions are checked before the object is retrieved** + + A user without permission to view objects is now refused before anything is requested from the Objecten API. + Previously the object was retrieved first, so the answer of the Objecten API could tell such a user whether an + object exists.