diff --git a/.github/workflows/backend-tests.yml b/.github/workflows/backend-tests.yml index 2b8ad89139..7617869a13 100644 --- a/.github/workflows/backend-tests.yml +++ b/.github/workflows/backend-tests.yml @@ -16,10 +16,6 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 15 env: - CROSSREF_USERNAME: ${{ secrets.CROSSREF_USERNAME }} - CROSSREF_PASSWORD: ${{ secrets.CROSSREF_PASSWORD }} - CROSSREF_ENDPOINT: ${{ secrets.CROSSREF_ENDPOINT }} - CROSSREF_DOI_PREFIX: ${{ secrets.CROSSREF_DOI_PREFIX }} RUN_EXTRA_TESTS: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' && 'true' || 'false' }} steps: - name: Login to Docker Hub diff --git a/backend/src/main/kotlin/org/loculus/backend/service/crossref/CrossRefService.kt b/backend/src/main/kotlin/org/loculus/backend/service/crossref/CrossRefService.kt index 57cb1c6ec3..f64c1072af 100644 --- a/backend/src/main/kotlin/org/loculus/backend/service/crossref/CrossRefService.kt +++ b/backend/src/main/kotlin/org/loculus/backend/service/crossref/CrossRefService.kt @@ -35,6 +35,7 @@ data class CrossRefServiceProperties( val email: String?, val organization: String?, val hostUrl: String?, + val writeEnabled: Boolean?, ) data class DoiEntry( @@ -54,14 +55,15 @@ data class CrossRefCitedByResult( @Service class CrossRefService(private val properties: CrossRefServiceProperties, private val dateProvider: DateProvider) { - val isActive = properties.endpoint != null && - properties.username != null && - properties.password != null && - properties.doiPrefix != null && - properties.databaseName != null && - properties.email != null && - properties.organization != null && - properties.hostUrl != null + val isActive = !properties.endpoint.isNullOrBlank() && + !properties.username.isNullOrBlank() && + !properties.password.isNullOrBlank() && + !properties.doiPrefix.isNullOrBlank() && + !properties.databaseName.isNullOrBlank() && + !properties.email.isNullOrBlank() && + !properties.organization.isNullOrBlank() && + !properties.hostUrl.isNullOrBlank() + val isWriteEnabled = properties.writeEnabled == true val doiPrefix: String? = properties.doiPrefix val dateTimeFormatterMM: DateTimeFormatter = DateTimeFormatter.ofPattern("MM") val dateTimeFormatterdd: DateTimeFormatter = DateTimeFormatter.ofPattern("dd") @@ -73,6 +75,12 @@ class CrossRefService(private val properties: CrossRefServiceProperties, private } } + private fun checkIsWriteEnabled() { + if (!isWriteEnabled) { + throw RuntimeException("The CrossRefService is read-only so this action is not permitted.") + } + } + fun parseCrossRefCitedByXML(citedByXML: String): CrossRefCitedByResult { val parser = Parser.xmlParser().setTrackErrors(1) val doc = Jsoup.parse(citedByXML, "", parser) @@ -280,6 +288,7 @@ class CrossRefService(private val properties: CrossRefServiceProperties, private fun postCrossRefXML(XML: String): String { checkIsActive() + checkIsWriteEnabled() // This is needed per their API specification val formData = mapOf( diff --git a/backend/src/main/kotlin/org/loculus/backend/service/seqsetcitations/SeqSetCitationsDatabaseService.kt b/backend/src/main/kotlin/org/loculus/backend/service/seqsetcitations/SeqSetCitationsDatabaseService.kt index 55fe3cd92e..93a23233fb 100644 --- a/backend/src/main/kotlin/org/loculus/backend/service/seqsetcitations/SeqSetCitationsDatabaseService.kt +++ b/backend/src/main/kotlin/org/loculus/backend/service/seqsetcitations/SeqSetCitationsDatabaseService.kt @@ -35,6 +35,7 @@ import org.loculus.backend.api.Status.APPROVED_FOR_RELEASE import org.loculus.backend.api.SubmittedSeqSetRecord import org.loculus.backend.auth.AuthenticatedUser import org.loculus.backend.config.BackendConfig +import org.loculus.backend.controller.ForbiddenException import org.loculus.backend.controller.NotFoundException import org.loculus.backend.controller.UnprocessableEntityException import org.loculus.backend.service.crossref.CrossRefService @@ -107,7 +108,7 @@ class SeqSetCitationsDatabaseService( } return ResponseSeqSet( - insertedSet[SeqSetsTable.seqSetId].toString(), + insertedSet[SeqSetsTable.seqSetId], insertedSet[SeqSetsTable.seqSetVersion], ) } @@ -186,7 +187,7 @@ class SeqSetCitationsDatabaseService( } return ResponseSeqSet( - insertedSet[SeqSetsTable.seqSetId].toString(), + insertedSet[SeqSetsTable.seqSetId], insertedSet[SeqSetsTable.seqSetVersion], ) } @@ -342,6 +343,14 @@ class SeqSetCitationsDatabaseService( val username = authenticatedUser.username log.info { "Create DOI for seqSet $seqSetId, version $version, user $username" } + if (!crossRefService.isActive) { + throw ForbiddenException("The crossref service is not active, so creating DOIs is forbidden.") + } + + if (!crossRefService.isWriteEnabled) { + throw ForbiddenException("The crossref service is not write-enabled, so creating DOIs is forbidden.") + } + validateCreateSeqSetDOI(username, seqSetId, version) val doiPrefix = crossRefService.doiPrefix ?: "no-prefix-configured" @@ -359,18 +368,16 @@ class SeqSetCitationsDatabaseService( it[SeqSetsTable.seqSetDOI] = seqSetDOI } - if (crossRefService.isActive) { - val crossRefXml = crossRefService.generateCrossRefXML( - DoiEntry( - LocalDate.now(), - "SeqSet: ${seqSet[0].name}", - seqSetDOI, - "/seqsets/$seqSetId.$version", - null, - ), - ) - crossRefService.postCrossRefXML(crossRefXml) - } + val crossRefXml = crossRefService.generateCrossRefXML( + DoiEntry( + LocalDate.now(), + "SeqSet: ${seqSet[0].name}", + seqSetDOI, + "/seqsets/$seqSetId.$version", + null, + ), + ) + crossRefService.postCrossRefXML(crossRefXml) return ResponseSeqSet( seqSetId, @@ -474,10 +481,10 @@ class SeqSetCitationsDatabaseService( mutableListOf(), ) - val uniqueSeqSetIds = latestSeqSetWithUserAccession.map { it.seqSetId.toString() }.toSet() + val uniqueSeqSetIds = latestSeqSetWithUserAccession.map { it.seqSetId }.toSet() for (seqSetId in uniqueSeqSetIds) { val year = latestSeqSetWithUserAccession - .first { it.seqSetId.toString() == seqSetId } + .first { it.seqSetId == seqSetId } .createdAt.toLocalDateTime().year.toLong() if (citedBy.years.contains(year)) { val index = citedBy.years.indexOf(year) diff --git a/backend/src/test/kotlin/org/loculus/backend/controller/seqsetcitations/CitationEndpointsTest.kt b/backend/src/test/kotlin/org/loculus/backend/controller/seqsetcitations/CitationEndpointsTest.kt index de4fe7456c..0684d3c15a 100644 --- a/backend/src/test/kotlin/org/loculus/backend/controller/seqsetcitations/CitationEndpointsTest.kt +++ b/backend/src/test/kotlin/org/loculus/backend/controller/seqsetcitations/CitationEndpointsTest.kt @@ -50,7 +50,10 @@ class CitationEndpointsTest( } returns listOf(AccessionVersion(MOCK_SEQ_ACCESSION, MOCK_SEQ_VERSION)) every { accessionPreconditionValidator.validate(any()) } returns Unit every { crossRefService.doiPrefix } returns MOCK_DOI_PREFIX - every { crossRefService.isActive } returns false + every { crossRefService.isActive } returns true + every { crossRefService.isWriteEnabled } returns true + every { crossRefService.generateCrossRefXML(any()) } returns "" + every { crossRefService.postCrossRefXML(any()) } returns "Crossref API response" } @ParameterizedTest @@ -117,7 +120,6 @@ class CitationEndpointsTest( val seqSetVersion = JsonPath.read(seqSetResult.response.contentAsString, "$.seqSetVersion").toLong() // Simulate running the crossref citations task - every { crossRefService.isActive } returns true every { crossRefService.getCrossRefCitedBy(MOCK_DOI_PREFIX) } returns CrossRefCitedByResult(emptyList(), emptyList()) seqSetCrossRefCitationsTask.task() @@ -157,7 +159,6 @@ class CitationEndpointsTest( ), seqSetDOIs = setOf(seqSetDOI), ) - every { crossRefService.isActive } returns true every { crossRefService.getCrossRefCitedBy(MOCK_DOI_PREFIX) } returns CrossRefCitedByResult(listOf(seqSetCitationSource), emptyList()) seqSetCrossRefCitationsTask.task() @@ -204,8 +205,6 @@ class CitationEndpointsTest( val (seqSetIdA, seqSetVersionA, seqSetDOIA) = createSeqSetWithDOI() val (seqSetIdB, seqSetVersionB, seqSetDOIB) = createSeqSetWithDOI() - every { crossRefService.isActive } returns true - val citationSource = SeqSetCitationSource( CitationSource( sourceDOI = "10.5678/citing-paper", diff --git a/backend/src/test/kotlin/org/loculus/backend/controller/seqsetcitations/SeqSetEndpointsTest.kt b/backend/src/test/kotlin/org/loculus/backend/controller/seqsetcitations/SeqSetEndpointsTest.kt index f09b6f73b8..9acf9f9662 100644 --- a/backend/src/test/kotlin/org/loculus/backend/controller/seqsetcitations/SeqSetEndpointsTest.kt +++ b/backend/src/test/kotlin/org/loculus/backend/controller/seqsetcitations/SeqSetEndpointsTest.kt @@ -31,7 +31,10 @@ class SeqSetEndpointsTest(@Autowired private val client: SeqSetCitationsControll fun setup() { every { accessionPreconditionValidator.validate(any()) } returns Unit every { crossRefService.doiPrefix } returns MOCK_DOI_PREFIX - every { crossRefService.isActive } returns false + every { crossRefService.isActive } returns true + every { crossRefService.isWriteEnabled } returns true + every { crossRefService.generateCrossRefXML(any()) } returns "" + every { crossRefService.postCrossRefXML(any()) } returns "Crossref API response" } @ParameterizedTest @@ -122,6 +125,36 @@ class SeqSetEndpointsTest(@Autowired private val client: SeqSetCitationsControll .andExpect(jsonPath("\$[0].seqSetDOI").isString) } + @Test + fun `WHEN calling create seqSet DOI while write is disabled THEN returns forbidden`() { + every { crossRefService.isWriteEnabled } returns false + + val seqSetResult = client.createSeqSet() + .andExpect(status().isOk) + .andReturn() + val seqSetId = JsonPath.read(seqSetResult.response.contentAsString, "$.seqSetId") + + client.createSeqSetDOI(seqSetId) + .andExpect(status().isForbidden) + .andExpect(content().contentType(MediaType.APPLICATION_PROBLEM_JSON)) + .andExpect(jsonPath("\$.detail", containsString("not write-enabled"))) + } + + @Test + fun `WHEN calling create seqSet DOI while crossref is inactive THEN returns forbidden`() { + every { crossRefService.isActive } returns false + + val seqSetResult = client.createSeqSet() + .andExpect(status().isOk) + .andReturn() + val seqSetId = JsonPath.read(seqSetResult.response.contentAsString, "$.seqSetId") + + client.createSeqSetDOI(seqSetId) + .andExpect(status().isForbidden) + .andExpect(content().contentType(MediaType.APPLICATION_PROBLEM_JSON)) + .andExpect(jsonPath("\$.detail", containsString("not active"))) + } + @Test fun `WHEN calling update seqSet THEN returns updated seqSet`() { val seqSetResult = client.createSeqSet() diff --git a/backend/src/test/kotlin/org/loculus/backend/controller/seqsetcitations/SeqSetValidationEndpointsTest.kt b/backend/src/test/kotlin/org/loculus/backend/controller/seqsetcitations/SeqSetValidationEndpointsTest.kt index 3c4fd8e1fa..3c2f59a932 100644 --- a/backend/src/test/kotlin/org/loculus/backend/controller/seqsetcitations/SeqSetValidationEndpointsTest.kt +++ b/backend/src/test/kotlin/org/loculus/backend/controller/seqsetcitations/SeqSetValidationEndpointsTest.kt @@ -30,7 +30,10 @@ class SeqSetValidationEndpointsTest( @BeforeEach fun setup() { every { crossRefService.doiPrefix } returns MOCK_DOI_PREFIX - every { crossRefService.isActive } returns false + every { crossRefService.isActive } returns true + every { crossRefService.isWriteEnabled } returns true + every { crossRefService.generateCrossRefXML(any()) } returns "" + every { crossRefService.postCrossRefXML(any()) } returns "Crossref API response" } @Test diff --git a/backend/src/test/kotlin/org/loculus/backend/service/crossref/CrossRefServiceTest.kt b/backend/src/test/kotlin/org/loculus/backend/service/crossref/CrossRefServiceTest.kt index 7aedc46fe2..d5d7b055f6 100644 --- a/backend/src/test/kotlin/org/loculus/backend/service/crossref/CrossRefServiceTest.kt +++ b/backend/src/test/kotlin/org/loculus/backend/service/crossref/CrossRefServiceTest.kt @@ -1,6 +1,7 @@ package org.loculus.backend.service.crossref import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertFalse import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertThrows @@ -10,6 +11,7 @@ import org.loculus.backend.SpringBootTestWithoutDatabase import org.loculus.backend.api.CitationContributor import org.loculus.backend.api.CitationSource import org.loculus.backend.api.SeqSetCitationSource +import org.loculus.backend.utils.DateProvider import org.springframework.beans.factory.annotation.Autowired import java.time.Instant import java.time.LocalDate @@ -30,6 +32,21 @@ class CrossRefServiceTest(@Autowired private val crossRefService: CrossRefServic $doiBatchID1711411200000Loculus Databasedois@loculus.orgLoculus DatabaseLoculus Databaseloculus.orgSeqSet: My test set03262024$doihttps://main.loculus.org/seqsets/LOC_SS_1.1 """.trimIndent() + private fun crossRefService(writeEnabled: Boolean?) = CrossRefService( + CrossRefServiceProperties( + endpoint = "dummy", + username = "dummy", + password = "dummy", + doiPrefix = "placeholder", + databaseName = "Loculus Database", + email = "dois@loculus.org", + organization = "loculus.org", + hostUrl = "https://main.loculus.org", + writeEnabled = writeEnabled, + ), + DateProvider(), + ) + @Test fun `parseCrossRefCitedByXML returns citations from valid XML across different citation types`() { val xml = """ @@ -270,6 +287,21 @@ class CrossRefServiceTest(@Autowired private val crossRefService: CrossRefServic ) } + @Test + fun `postCrossRefXML is rejected when write is not enabled`() { + val readOnlyService = crossRefService(writeEnabled = false) + val ex = assertThrows { + readOnlyService.postCrossRefXML(crossRefXMLReference) + } + assertTrue(ex.message!!.contains("read-only", ignoreCase = true)) + } + + @Test + fun `crossref write-enabled=true property string is coerced to the boolean flag`() { + // Application properties sets crossref.write-enabled=true + assertTrue(crossRefService.isWriteEnabled) + } + @Test fun `Create an XML metadata string complying with CrossRef's schema`() { val crossRefXML = crossRefService.generateCrossRefXML( diff --git a/backend/src/test/resources/application.properties b/backend/src/test/resources/application.properties index 4961ec5b3e..2eae6c6bda 100644 --- a/backend/src/test/resources/application.properties +++ b/backend/src/test/resources/application.properties @@ -10,6 +10,7 @@ crossref.database-name=Loculus Database crossref.email=dois@loculus.org crossref.organization=loculus.org crossref.host-url=https://main.loculus.org +crossref.write-enabled=true keycloak.user=dummy keycloak.password=dummy diff --git a/kubernetes/loculus/templates/loculus-backend.yaml b/kubernetes/loculus/templates/loculus-backend.yaml index a6d0bedad3..1a0db9b700 100644 --- a/kubernetes/loculus/templates/loculus-backend.yaml +++ b/kubernetes/loculus/templates/loculus-backend.yaml @@ -58,6 +58,7 @@ spec: - "--crossref.email=$(CROSSREF_EMAIL)" - "--crossref.organization=$(CROSSREF_ORGANIZATION)" - "--crossref.host-url=$(CROSSREF_HOST_URL)" + - "--crossref.write-enabled=$(CROSSREF_WRITE_ENABLED)" {{- end }} - "--keycloak.password=$(BACKEND_KEYCLOAK_PASSWORD)" - "--keycloak.realm=loculus" @@ -108,6 +109,8 @@ spec: value: {{$.Values.seqSets.crossRef.organization | quote }} - name: CROSSREF_HOST_URL value: {{$.Values.seqSets.crossRef.hostUrl | quote }} + - name: CROSSREF_WRITE_ENABLED + value: {{$.Values.seqSets.crossRef.writeEnabled | quote }} {{- end }} - name: BACKEND_KEYCLOAK_PASSWORD valueFrom: diff --git a/kubernetes/loculus/values.schema.json b/kubernetes/loculus/values.schema.json index fcb96fd10c..9f0b4ac884 100644 --- a/kubernetes/loculus/values.schema.json +++ b/kubernetes/loculus/values.schema.json @@ -1286,6 +1286,12 @@ "groups": ["general"], "type": "string", "description": "The host URL for CrossRef callbacks" + }, + "writeEnabled": { + "groups": ["general"], + "type": "boolean", + "default": false, + "description": "Enable/disable Crossref write functionality, such as generating DOIs" } } },