From f593073c8571f295af0ca0c6fcdcb8da7a9143aa Mon Sep 17 00:00:00 2001 From: Fabian Engelniederhammer Date: Mon, 15 Jan 2024 08:37:19 +0100 Subject: [PATCH] feat: fix authentication, don't document authentication for now #553 Also requests like "fields=[]" should be considered to return detailed data. We decided not to document authentication for now. The current implementation is special logic required for the GISAID instance. It might be removed in the future. Anyway, if someone else need authentication on LAPIS, we should implement something better. --- lapis2-docs/astro.config.mjs | 4 --- .../references/database-configuration.mdx | 13 ++++--- .../docs/references/authentication.mdx | 10 ------ lapis2-docs/tests/docs.spec.ts | 1 - .../auth/DataOpennessAuthorizationFilter.kt | 13 +++++-- .../resources/application-docker.properties | 1 + .../auth/ProtectedDataAuthorizationTest.kt | 35 ++++++++++++++++++- 7 files changed, 52 insertions(+), 25 deletions(-) delete mode 100644 lapis2-docs/src/content/docs/references/authentication.mdx diff --git a/lapis2-docs/astro.config.mjs b/lapis2-docs/astro.config.mjs index 8fb2cc94..29e84b5f 100644 --- a/lapis2-docs/astro.config.mjs +++ b/lapis2-docs/astro.config.mjs @@ -58,10 +58,6 @@ export default defineConfig({ label: 'Database Config', link: '/references/database-config/', }, - { - label: 'Authentication', - link: '/references/authentication/', - }, ], }, { diff --git a/lapis2-docs/src/content/docs/maintainer-docs/references/database-configuration.mdx b/lapis2-docs/src/content/docs/maintainer-docs/references/database-configuration.mdx index c58ed18d..f6fe1d56 100644 --- a/lapis2-docs/src/content/docs/maintainer-docs/references/database-configuration.mdx +++ b/lapis2-docs/src/content/docs/maintainer-docs/references/database-configuration.mdx @@ -24,7 +24,7 @@ It permits the following fields: | ------------- | ------ | -------- | ----------------------------------------------------------------------------------------------------- | | instanceName | string | true | The name assigned to the instance. Only used for diplay purposes. | | metadata | array | true | A list of [metadata objects](#the-metadata-object) that is available on the underlying sequence data. | -| opennessLevel | enum | true | `OPEN` or `PROTECTED`. See [authentication](/references/authentication). | +| opennessLevel | enum | true | Possible values: `OPEN`. To be extended in the future. | | primaryKey | string | true | The field that serves as the primary key in SILO for the data. | | dateToSortBy | string | false | The field used to sort the data by date. Queries on this column will be faster. | | partitionBy | string | false | The field used to partition the data. Used by SILO for overall query optimization. | @@ -45,12 +45,11 @@ it will be beneficial to set `dateToSortBy` to that column. The metadata object permits the following fields: -| Key | Type | Required | Description | -| --------------- | ------- | -------- | -------------------------------------------------------------------------------------------------------------------------------------- | -| name | string | true | The name of the feature. | -| type | enum | true | The [type of the metadata](#metadata-types). | -| valuesAreUnique | boolean | false | Whether this column uniquely identifies a row. Important when using the `PROTECTED` [authentication](/references/authentication) mode. | -| generateIndex | boolean | false | Some [metadata types](#metadata-types) permit generating an index for faster queries on the column. | +| Key | Type | Required | Description | +| ------------- | ------- | -------- | --------------------------------------------------------------------------------------------------- | +| name | string | true | The name of the feature. | +| type | enum | true | The [type of the metadata](#metadata-types). | +| generateIndex | boolean | false | Some [metadata types](#metadata-types) permit generating an index for faster queries on the column. | ### Metadata Types diff --git a/lapis2-docs/src/content/docs/references/authentication.mdx b/lapis2-docs/src/content/docs/references/authentication.mdx deleted file mode 100644 index 86146b55..00000000 --- a/lapis2-docs/src/content/docs/references/authentication.mdx +++ /dev/null @@ -1,10 +0,0 @@ ---- -title: Authentication -description: TODO ADD A DESCRIPTION ---- - -TODO https://github.com/GenSpectrum/LAPIS/issues/553 - -:::caution -write something about setting `valuesAreUnique` of metadata fields -::: diff --git a/lapis2-docs/tests/docs.spec.ts b/lapis2-docs/tests/docs.spec.ts index 6c2de6c3..86017018 100644 --- a/lapis2-docs/tests/docs.spec.ts +++ b/lapis2-docs/tests/docs.spec.ts @@ -10,7 +10,6 @@ const referencesPages = [ 'Open API / Swagger', 'Reference Genome', 'Database Config', - 'Authentication', ]; const conceptsPages = [ diff --git a/lapis2/src/main/kotlin/org/genspectrum/lapis/auth/DataOpennessAuthorizationFilter.kt b/lapis2/src/main/kotlin/org/genspectrum/lapis/auth/DataOpennessAuthorizationFilter.kt index 2d4de5cf..bdb31ccd 100644 --- a/lapis2/src/main/kotlin/org/genspectrum/lapis/auth/DataOpennessAuthorizationFilter.kt +++ b/lapis2/src/main/kotlin/org/genspectrum/lapis/auth/DataOpennessAuthorizationFilter.kt @@ -12,10 +12,13 @@ import org.genspectrum.lapis.controller.ACCESS_KEY_PROPERTY import org.genspectrum.lapis.controller.AGGREGATED_ROUTE import org.genspectrum.lapis.controller.AMINO_ACID_INSERTIONS_ROUTE import org.genspectrum.lapis.controller.AMINO_ACID_MUTATIONS_ROUTE +import org.genspectrum.lapis.controller.DATABASE_CONFIG_ROUTE +import org.genspectrum.lapis.controller.FIELDS_PROPERTY import org.genspectrum.lapis.controller.INFO_ROUTE import org.genspectrum.lapis.controller.LapisErrorResponse import org.genspectrum.lapis.controller.NUCLEOTIDE_INSERTIONS_ROUTE import org.genspectrum.lapis.controller.NUCLEOTIDE_MUTATIONS_ROUTE +import org.genspectrum.lapis.controller.REFERENCE_GENOME_ROUTE import org.genspectrum.lapis.util.CachedBodyHttpServletRequest import org.springframework.http.HttpStatus import org.springframework.http.MediaType @@ -97,7 +100,12 @@ private class ProtectedDataAuthorizationFilter( ) : DataOpennessAuthorizationFilter(objectMapper) { companion object { - private val WHITELISTED_PATH_PREFIXES = listOf("/swagger-ui", "/api-docs") + private val WHITELISTED_PATH_PREFIXES = listOf( + "/swagger-ui", + "/api-docs", + "/sample$DATABASE_CONFIG_ROUTE", + "/sample$REFERENCE_GENOME_ROUTE", + ) private val ENDPOINTS_THAT_SERVE_AGGREGATED_DATA = listOf( AGGREGATED_ROUTE, NUCLEOTIDE_MUTATIONS_ROUTE, @@ -129,7 +137,8 @@ private class ProtectedDataAuthorizationFilter( } val endpointServesAggregatedData = ENDPOINTS_THAT_SERVE_AGGREGATED_DATA.contains(path) && - fieldsThatServeNonAggregatedData.intersect(requestFields.keys).isEmpty() + fieldsThatServeNonAggregatedData.intersect(requestFields.keys).isEmpty() && + requestFields[FIELDS_PROPERTY]?.intersect(fieldsThatServeNonAggregatedData.toSet())?.isNotEmpty() != false if (endpointServesAggregatedData && accessKeys.aggregatedDataAccessKey == accessKey) { return AuthorizationResult.success() diff --git a/lapis2/src/main/resources/application-docker.properties b/lapis2/src/main/resources/application-docker.properties index 395b53cc..c3e19fc4 100644 --- a/lapis2/src/main/resources/application-docker.properties +++ b/lapis2/src/main/resources/application-docker.properties @@ -1 +1,2 @@ lapis.databaseConfig.path=./database_config.yaml +lapis.accessKeys.path=./access_keys.yaml diff --git a/lapis2/src/test/kotlin/org/genspectrum/lapis/auth/ProtectedDataAuthorizationTest.kt b/lapis2/src/test/kotlin/org/genspectrum/lapis/auth/ProtectedDataAuthorizationTest.kt index 3f847754..6667b2ab 100644 --- a/lapis2/src/test/kotlin/org/genspectrum/lapis/auth/ProtectedDataAuthorizationTest.kt +++ b/lapis2/src/test/kotlin/org/genspectrum/lapis/auth/ProtectedDataAuthorizationTest.kt @@ -6,6 +6,8 @@ import io.mockk.every import io.mockk.verify import org.genspectrum.lapis.PRIMARY_KEY_FIELD import org.genspectrum.lapis.controller.AGGREGATED_ROUTE +import org.genspectrum.lapis.controller.DATABASE_CONFIG_ROUTE +import org.genspectrum.lapis.controller.REFERENCE_GENOME_ROUTE import org.genspectrum.lapis.controller.getSample import org.genspectrum.lapis.controller.postSample import org.genspectrum.lapis.model.SiloQueryModel @@ -148,6 +150,31 @@ 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`() { + mockMvc.perform( + getSample("$validRoute?accessKey=testAggregatedDataAccessKey&fields=$PRIMARY_KEY_FIELD"), + ) + .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`() { + mockMvc.perform( + postRequestWithBody( + """ { + "accessKey": "testAggregatedDataAccessKey", + "fields": ["$PRIMARY_KEY_FIELD"] + }""", + ), + ) + .andExpect(status().isForbidden) + .andExpect(content().contentType(MediaType.APPLICATION_JSON)) + .andExpect(content().json(NOT_AUTHORIZED_TO_ACCESS_ENDPOINT_ERROR)) + } + @Test fun `given valid access key for full access in GET request to protected instance, then access is granted`() { mockMvc.perform( @@ -186,7 +213,7 @@ class ProtectedDataAuthorizationTest( ) @Test - fun `the swagger ui and api docs are always accessible`() { + fun `whitelisted routes are always accessible`() { mockMvc.perform(get("/swagger-ui/index.html")) .andExpect(status().isOk) @@ -195,6 +222,12 @@ class ProtectedDataAuthorizationTest( mockMvc.perform(get("/api-docs.yaml")) .andExpect(status().isOk) + + mockMvc.perform(getSample(DATABASE_CONFIG_ROUTE)) + .andExpect(status().isOk) + + mockMvc.perform(getSample(REFERENCE_GENOME_ROUTE)) + .andExpect(status().isOk) } private fun postRequestWithBody(body: String) =