fix: allow access in authorized mode when fields is specified non-f…

…ine-grained

Before, `intersect` was called on a JsonNode, which did not work as expected.

lapis2/src/main/kotlin/org/genspectrum/lapis/auth/DataOpennessAuthorizationFilter.kt

@@ -123,18 +123,17 @@ private class ProtectedDataAuthorizationFilter(
             return AuthorizationResult.success()
         }
 
-        val requestFields = request.getRequestFields()
-
-        val accessKey = requestFields[ACCESS_KEY_PROPERTY]?.textValue()
+        val accessKey = request.getStringField(ACCESS_KEY_PROPERTY)
             ?: return AuthorizationResult.failure("An access key is required to access $path.")
 
         if (accessKeys.fullAccessKey == accessKey) {
             return AuthorizationResult.success()
         }
 
         val endpointServesAggregatedData = ENDPOINTS_THAT_SERVE_AGGREGATED_DATA.contains(path) &&
-            fieldsThatServeNonAggregatedData.intersect(requestFields.keys).isEmpty() &&
-            requestFields[FIELDS_PROPERTY]?.intersect(fieldsThatServeNonAggregatedData.toSet())?.isNotEmpty() != false
+            fieldsThatServeNonAggregatedData.intersect(request.getRequestFieldNames()).isEmpty() &&
+            request.getStringArrayField(FIELDS_PROPERTY).intersect(fieldsThatServeNonAggregatedData.toSet())
+                .isEmpty()
 
         if (endpointServesAggregatedData && accessKeys.aggregatedDataAccessKey == accessKey) {
             return AuthorizationResult.success()

lapis2/src/main/kotlin/org/genspectrum/lapis/controller/DataFormatParameterFilter.kt

@@ -40,7 +40,7 @@ class AcceptHeaderModifyingRequestWrapper(
 ) : HttpServletRequestWrapper(reReadableRequest) {
     override fun getHeader(name: String): String? {
         if (name.equals("Accept", ignoreCase = true)) {
-            when (reReadableRequest.getRequestFields()[FORMAT_PROPERTY]?.textValue()?.uppercase()) {
+            when (reReadableRequest.getStringField(FORMAT_PROPERTY)?.uppercase()) {
                 "CSV" -> {
                     log.debug { "Overwriting Accept header to $TEXT_CSV_HEADER due to format property" }
                     return TEXT_CSV_HEADER
@@ -67,7 +67,7 @@ class AcceptHeaderModifyingRequestWrapper(
 
     override fun getHeaders(name: String): Enumeration<String>? {
         if (name.equals("Accept", ignoreCase = true)) {
-            when (reReadableRequest.getRequestFields()[FORMAT_PROPERTY]?.textValue()?.uppercase()) {
+            when (reReadableRequest.getStringField(FORMAT_PROPERTY)?.uppercase()) {
                 "CSV" -> {
                     log.debug { "Overwriting Accept header to $TEXT_CSV_HEADER due to format property" }
                     return Collections.enumeration(listOf(TEXT_CSV_HEADER))

lapis2/src/main/kotlin/org/genspectrum/lapis/controller/DownloadAsFileFilter.kt

@@ -20,8 +20,8 @@ class DownloadAsFileFilter(private val objectMapper: ObjectMapper) : OncePerRequ
     ) {
         val reReadableRequest = CachedBodyHttpServletRequest(request, objectMapper)
 
-        val downloadAsFile = reReadableRequest.getRequestFields()[DOWNLOAD_AS_FILE_PROPERTY]?.asBoolean()
-        if (downloadAsFile == true) {
+        val downloadAsFile = reReadableRequest.getBooleanField(DOWNLOAD_AS_FILE_PROPERTY) ?: false
+        if (downloadAsFile) {
             val filename = getFilename(reReadableRequest)
             response.setHeader(HttpHeaders.CONTENT_DISPOSITION, "attachment; filename=$filename")
         }

lapis2/src/main/kotlin/org/genspectrum/lapis/util/CachedBodyHttpServletRequest.kt

@@ -2,7 +2,6 @@ package org.genspectrum.lapis.util
 
 import com.fasterxml.jackson.databind.JsonNode
 import com.fasterxml.jackson.databind.ObjectMapper
-import com.fasterxml.jackson.databind.node.TextNode
 import com.fasterxml.jackson.module.kotlin.readValue
 import jakarta.servlet.ReadListener
 import jakarta.servlet.ServletInputStream
@@ -27,6 +26,22 @@ class CachedBodyHttpServletRequest(request: HttpServletRequest, val objectMapper
         byteArrayOutputStream.toByteArray()
     }
 
+    private val parsedBody: Map<String, JsonNode> by lazy {
+        try {
+            objectMapper.readValue(inputStream)
+        } catch (exception: Exception) {
+            if (method != HttpMethod.GET.name()) {
+                log.warn { "Failed to read from request body: ${exception.message}, contentLength $contentLength" }
+                log.debug { exception.stackTraceToString() }
+            } else {
+                log.warn {
+                    "Tried to read request body of GET request: ${exception.message}, contentLength $contentLength"
+                }
+            }
+            emptyMap()
+        }
+    }
+
     @Throws(IOException::class)
     override fun getInputStream(): ServletInputStream {
         return CachedBodyServletInputStream(ByteArrayInputStream(cachedBody))
@@ -60,24 +75,46 @@ class CachedBodyHttpServletRequest(request: HttpServletRequest, val objectMapper
         }
     }
 
-    fun getRequestFields(): Map<String, JsonNode> {
-        if (parameterNames.hasMoreElements()) {
-            return parameterMap.mapValues { (_, value) -> TextNode(value.joinToString()) }
+    fun getStringField(fieldName: String): String? {
+        if (method == HttpMethod.GET.name()) {
+            return parameterMap[fieldName]?.firstOrNull()
         }
 
-        if (contentLength == 0) {
-            log.debug { "Could not read from request body, because content length is 0." }
-            return emptyMap()
+        val fieldValue = parsedBody[fieldName]
+        if (fieldValue?.isTextual == true) {
+            return fieldValue.asText()
         }
+        return null
+    }
 
-        return try {
-            objectMapper.readValue(inputStream)
-        } catch (exception: Exception) {
-            if (method != HttpMethod.GET.name()) {
-                log.warn { "Failed to read from request body: ${exception.message}, contentLength $contentLength" }
-                log.debug { exception.stackTraceToString() }
-            }
-            emptyMap()
+    fun getBooleanField(fieldName: String): Boolean? {
+        if (method == HttpMethod.GET.name()) {
+            return parameterMap[fieldName]?.firstOrNull()?.toBooleanStrictOrNull()
+        }
+
+        val fieldValue = parsedBody[fieldName]
+        if (fieldValue?.isBoolean == true) {
+            return fieldValue.asBoolean()
+        }
+        return null
+    }
+
+    fun getStringArrayField(fieldName: String): List<String> {
+        if (method == HttpMethod.GET.name()) {
+            return parameterMap[fieldName]?.flatMap { it.split(',') } ?: emptyList()
+        }
+
+        val fieldValue = parsedBody[fieldName]
+        if (fieldValue?.isArray == true) {
+            return fieldValue.map { it.asText() }
+        }
+        return emptyList()
+    }
+
+    fun getRequestFieldNames(): Set<String> {
+        return when (method) {
+            HttpMethod.GET.name() -> parameterMap.keys
+            else -> parsedBody.keys
         }
     }
 }

lapis2/src/test/kotlin/org/genspectrum/lapis/auth/ProtectedDataAuthorizationTest.kt

@@ -15,6 +15,8 @@ import org.genspectrum.lapis.request.LapisInfo
 import org.genspectrum.lapis.request.SequenceFiltersRequestWithFields
 import org.junit.jupiter.api.BeforeEach
 import org.junit.jupiter.api.Test
+import org.junit.jupiter.params.ParameterizedTest
+import org.junit.jupiter.params.provider.ValueSource
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc
 import org.springframework.boot.test.context.SpringBootTest
@@ -150,23 +152,49 @@ class ProtectedDataAuthorizationTest(
             .andExpect(content().json(NOT_AUTHORIZED_TO_ACCESS_ENDPOINT_ERROR))
     }
 
-    @Test
-    fun `GIVEN aggregated access key in GET request but request stratifies too fine-grained THEN access is denied`() {
+    @ParameterizedTest
+    @ValueSource(
+        strings = [
+            "fields=$PRIMARY_KEY_FIELD",
+            "fields=$PRIMARY_KEY_FIELD,country",
+            "fields=$PRIMARY_KEY_FIELD&fields=country",
+        ],
+    )
+    fun `GIVEN aggregated access key in GET request but request stratifies too fine-grained THEN access is denied`(
+        fieldsQueryParameter: String,
+    ) {
         mockMvc.perform(
-            getSample("$validRoute?accessKey=testAggregatedDataAccessKey&fields=$PRIMARY_KEY_FIELD"),
+            getSample("$validRoute?accessKey=testAggregatedDataAccessKey&$fieldsQueryParameter"),
         )
             .andExpect(status().isForbidden)
             .andExpect(content().contentType(MediaType.APPLICATION_JSON))
             .andExpect(content().json(NOT_AUTHORIZED_TO_ACCESS_ENDPOINT_ERROR))
     }
 
     @Test
-    fun `GIVEN aggregated access key in POST request but request stratifies too fine-grained THEN access is denied`() {
+    fun `GIVEN aggregated access key in GET request that stratifies non-fine-grained THEN access is granted`() {
+        mockMvc.perform(
+            getSample("$validRoute?accessKey=testAggregatedDataAccessKey&fields=country"),
+        )
+            .andExpect(status().isOk)
+            .andExpect(content().contentType(MediaType.APPLICATION_JSON))
+    }
+
+    @ParameterizedTest
+    @ValueSource(
+        strings = [
+            """["$PRIMARY_KEY_FIELD"]""",
+            """["$PRIMARY_KEY_FIELD", "country"]""",
+        ],
+    )
+    fun `GIVEN aggregated access key in POST request but request stratifies too fine-grained THEN access is denied`(
+        fieldsJson: String,
+    ) {
         mockMvc.perform(
             postRequestWithBody(
                 """ {
                     "accessKey": "testAggregatedDataAccessKey",
-                    "fields": ["$PRIMARY_KEY_FIELD"]
+                    "fields": $fieldsJson
                 }""",
             ),
         )