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.42.0/README.md b/documentation/release-notes/13.x.x/13.42.0/README.md index 08c2706e61..31ca74c8a9 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,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