Skip to content

Commit

Permalink
feat: fix authentication, don't document authentication for now #553
Browse files Browse the repository at this point in the history
Also requests like "fields=[<primaryKey>]" 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.
  • Loading branch information
fengelniederhammer committed Jan 15, 2024
1 parent fab1c72 commit f593073
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 25 deletions.
4 changes: 0 additions & 4 deletions lapis2-docs/astro.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,6 @@ export default defineConfig({
label: 'Database Config',
link: '/references/database-config/',
},
{
label: 'Authentication',
link: '/references/authentication/',
},
],
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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. |
Expand All @@ -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

Expand Down
10 changes: 0 additions & 10 deletions lapis2-docs/src/content/docs/references/authentication.mdx

This file was deleted.

1 change: 0 additions & 1 deletion lapis2-docs/tests/docs.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ const referencesPages = [
'Open API / Swagger',
'Reference Genome',
'Database Config',
'Authentication',
];

const conceptsPages = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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()
Expand Down
1 change: 1 addition & 0 deletions lapis2/src/main/resources/application-docker.properties
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
lapis.databaseConfig.path=./database_config.yaml
lapis.accessKeys.path=./access_keys.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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)

Expand All @@ -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) =
Expand Down

0 comments on commit f593073

Please sign in to comment.