GH-581: Check object permissions before calling the Objecten API

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<ObjectWrapper>()!!
-
         authorizationService.requirePermission(
             EntityAuthorizationRequest(
                 Object::class.java,
@@ -65,6 +59,12 @@ class ObjectenApiClient(
             )
         )
 
+        val result = buildRestClient(authentication)
+            .get()
+            .uri(objectUrl)
+            .retrieve()
+            .body<ObjectWrapper>()!!
+
         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<ObjectRecord>()!!
 
-        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<ObjectsList>()!!
 
-        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<ObjectsList>()!!
 
-        authorizationService.requirePermission(
-            EntityAuthorizationRequest(
-                Object::class.java,
-                ObjectActionProvider.VIEW_LIST,
-                Object()
-            )
-        )
-
         outboxService.send {
             ObjectsListed(
                 objectMapper.valueToTree(result.results)

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>(any()) } doThrow AccessDeniedException("Unauthorized")
+            }
+            val client = ObjectenApiClient(restClientBuilder, outboxService, objectMapper, deniedAuthorizationService)
+
+            assertThrows<AccessDeniedException> { 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 ->

documentation/release-notes/13.x.x/13.42.0/README.md

@@ -18,7 +18,11 @@
 
 ## 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.
 
 ## Security