From a0fd84343ca7426b3f806f1b9d5b1962689bcb1e Mon Sep 17 00:00:00 2001 From: Andreas Svanberg <andreass@dsv.su.se> Date: Thu, 20 Feb 2025 13:56:13 +0100 Subject: [PATCH 1/3] Show error message when trying to add duplicate exemptions (#115) Fixes #62 ## How to test 1. Log in as admin 2. Go to "Admin / Match / Application periods" 3. Click "Edit exemptions" on the period 4. Add the same exemption twice to "Sture Student" Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/115 Reviewed-by: Nico Athanassiadis <nico@dsv.su.se> Co-authored-by: Andreas Svanberg <andreass@dsv.su.se> Co-committed-by: Andreas Svanberg <andreass@dsv.su.se> --- .../AdminEditApplicationPeriodExemptionsPage.properties | 1 + 1 file changed, 1 insertion(+) diff --git a/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodExemptionsPage.properties b/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodExemptionsPage.properties index bfb97053d6..21285f241c 100644 --- a/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodExemptionsPage.properties +++ b/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodExemptionsPage.properties @@ -10,3 +10,4 @@ Type.NUMBER_OF_AUTHORS=Min/max author limitation Type.SUBMIT_STUDENT_IDEA=Submit own idea outside open period Type.SELECT_SUPERVISOR_IDEA=Select supervisor idea outside open period type=Type to exempt +author.UniqueValidator=Exemption already exists, please remove the old one first From b7cf87d6d31cef80f06cef66f542838de4d34caa Mon Sep 17 00:00:00 2001 From: Andreas Svanberg <andreass@dsv.su.se> Date: Thu, 20 Feb 2025 14:31:59 +0100 Subject: [PATCH 2/3] Fix CVE-2024-57699 by override transitive dependency version (#116) The overriding should be removed once Spring Security updates its dependencies. Fixes #104 ## How to test 1. Run `mvnw install org.owasp:dependency-check-maven:12.1.0:check --fail-at-end -DnvdApiDelay=60000 -DskipTests -DfailBuildOnCVSS=7` 2. Wait a very long time (can be sped up be [requesting an NVD API key](https://nvd.nist.gov/developers/request-an-api-key) and adding `-DnvdApiKey=<key>` 3. Check the build succeeds Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/116 Reviewed-by: Nico Athanassiadis <nico@dsv.su.se> Co-authored-by: Andreas Svanberg <andreass@dsv.su.se> Co-committed-by: Andreas Svanberg <andreass@dsv.su.se> --- pom.xml | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/pom.xml b/pom.xml index 42018f99dc..a43a6491cd 100755 --- a/pom.xml +++ b/pom.xml @@ -39,6 +39,12 @@ <jersey.version>3.1.6</jersey.version> <poi.version>5.2.5</poi.version> <jackson.version>2.17.0</jackson.version> + + <!-- + When updating spring-boot check if the transitive dependency on json-smart has been + updated to 2.5.2 or later. + If so, remove the dependency managed version of json-smart + --> <spring.boot.version>3.4.1</spring.boot.version> <springdoc.openapi.version>2.8.3</springdoc.openapi.version> @@ -213,6 +219,20 @@ <version>2.0.2</version> <scope>runtime</scope> </dependency> + <dependency> + <!-- + 2.5.1 is brought in transitively by + spring-boot-starter-oauth2-client + spring-security-oauth2-client + oauth2-oidc-sdk + json-smart + it has a known security vulnerability that's fixed in 2.5.2 + should be removed when spring-boot-starter-oauth2-client is updated + --> + <groupId>net.minidev</groupId> + <artifactId>json-smart</artifactId> + <version>2.5.2</version> + </dependency> <!-- Test stuff --> <dependency> From 03570fc6db9bfbf82c256ffa33785d309718a836 Mon Sep 17 00:00:00 2001 From: Nico Athanassiadis <nico@dsv.su.se> Date: Fri, 21 Feb 2025 00:27:22 +0100 Subject: [PATCH 3/3] Improve supervisor change integration (#114) Previously when we wanted to change supervisor we had to make two calls against the daisy api. A DELETE and a POST, this was brittle because if one of the calls failed we didn't have a good way of handling that. This could leave the application in a state where a project could end up with 2 different supervisors. This caused side effects and forced us to manually go into the databases and clean up the errors. Now the daisy api is updated and we only need to do a POST to change the supervisor. See further documentation here [POST /thesis/{id}/contributor](https://apitest.dsv.su.se/resource_Theses.html#resource_Theses_postContributor_id_projectParticipant_POST) ### IMPORTANT: Release needs to be synced with Daisy API Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/114 Reviewed-by: Andreas Svanberg <andreass@dsv.su.se> Co-authored-by: Nico Athanassiadis <nico@dsv.su.se> Co-committed-by: Nico Athanassiadis <nico@dsv.su.se> --- .../dsv/scipro/reusable/SciProUtilities.java | 18 ++---- core/src/main/xjb/daisy_api.xjb | 4 ++ core/src/main/xsd/daisy_api.xsd | 18 ++++-- .../daisy/workers/ProjectExporter.java | 36 ++---------- .../io/impl/ExternalExporterDaisyImpl.java | 7 +-- .../daisy/workers/ProjectExporterTest.java | 56 ++++++++++++++++--- .../impl/ExternalExporterDaisyImplTest.java | 3 +- 7 files changed, 79 insertions(+), 63 deletions(-) diff --git a/core/src/main/java/se/su/dsv/scipro/reusable/SciProUtilities.java b/core/src/main/java/se/su/dsv/scipro/reusable/SciProUtilities.java index 0ed066a174..95f37a777c 100755 --- a/core/src/main/java/se/su/dsv/scipro/reusable/SciProUtilities.java +++ b/core/src/main/java/se/su/dsv/scipro/reusable/SciProUtilities.java @@ -1,7 +1,10 @@ package se.su.dsv.scipro.reusable; -import java.time.LocalDate; -import java.util.*; +import java.util.ArrayList; +import java.util.Calendar; +import java.util.Date; +import java.util.Iterator; +import java.util.List; public final class SciProUtilities { @@ -75,15 +78,4 @@ public final class SciProUtilities { } return copy; } - - public static Calendar toCalendar(final LocalDate localDate) { - if (localDate == null) { - return null; - } else { - final Calendar calendar = Calendar.getInstance(); - calendar.clear(); - calendar.set(localDate.getYear(), localDate.getMonthValue() - 1, localDate.getDayOfMonth()); - return calendar; - } - } } diff --git a/core/src/main/xjb/daisy_api.xjb b/core/src/main/xjb/daisy_api.xjb index a50ec543ca..5efd745a6a 100644 --- a/core/src/main/xjb/daisy_api.xjb +++ b/core/src/main/xjb/daisy_api.xjb @@ -9,6 +9,10 @@ parseMethod="jakarta.xml.bind.DatatypeConverter.parseDateTime" printMethod="jakarta.xml.bind.DatatypeConverter.printDateTime" /> + <jaxb:javaType name="java.time.LocalDate" xmlType="xs:date" + parseMethod="java.time.LocalDate.parse" + printMethod="java.time.format.DateTimeFormatter.ISO_LOCAL_DATE.format" /> + <!-- Force all classes implements Serializable --> <xjc:simple /> <xjc:serializable uid="1" /> diff --git a/core/src/main/xsd/daisy_api.xsd b/core/src/main/xsd/daisy_api.xsd index 0fa8ed93e9..fa68eef856 100755 --- a/core/src/main/xsd/daisy_api.xsd +++ b/core/src/main/xsd/daisy_api.xsd @@ -375,14 +375,16 @@ </xs:element> <xs:element name="title_en" type="xs:string" minOccurs="0"> </xs:element> - <xs:element name="startDate" type="xs:dateTime" minOccurs="0"> + <xs:element name="startDate" type="xs:date" minOccurs="0"> </xs:element> - <xs:element name="endDate" type="xs:dateTime" minOccurs="0"> + <xs:element name="endDate" type="xs:date" minOccurs="0"> </xs:element> <xs:element name="unit" type="serializableUnit" minOccurs="0"> </xs:element> <xs:element name="researchAreas" type="researchAreas" minOccurs="0"> </xs:element> + <xs:element name="externalID" type="xs:string" minOccurs="0"> + </xs:element> </xs:sequence> </xs:complexType> @@ -390,13 +392,15 @@ <xs:sequence> <xs:element name="title_en" type="xs:string" minOccurs="0"> </xs:element> + <xs:element name="externalID" type="xs:string" minOccurs="0"> + </xs:element> <xs:element name="aborted" type="xs:boolean" minOccurs="1"> </xs:element> <xs:element name="level" type="educationalLevel" minOccurs="0"> </xs:element> <xs:element name="title" type="xs:string" minOccurs="0"> </xs:element> - <xs:element name="endDate" type="xs:dateTime" minOccurs="0"> + <xs:element name="endDate" type="xs:date" minOccurs="0"> </xs:element> <xs:element name="status" type="STATUS" minOccurs="0"> </xs:element> @@ -404,7 +408,7 @@ </xs:element> <xs:element name="finished" type="xs:boolean" minOccurs="1"> </xs:element> - <xs:element name="startDate" type="xs:dateTime" minOccurs="0"> + <xs:element name="startDate" type="xs:date" minOccurs="0"> </xs:element> <xs:element name="id" type="xs:int" minOccurs="1"> </xs:element> @@ -475,9 +479,11 @@ </xs:element> <xs:element name="title_en" type="xs:string" minOccurs="0"> </xs:element> - <xs:element name="startDate" type="xs:dateTime" minOccurs="0"> + <xs:element name="externalID" type="xs:string" minOccurs="0"> </xs:element> - <xs:element name="endDate" type="xs:dateTime" minOccurs="0"> + <xs:element name="startDate" type="xs:date" minOccurs="0"> + </xs:element> + <xs:element name="endDate" type="xs:date" minOccurs="0"> </xs:element> <xs:element name="aborted" type="xs:boolean" minOccurs="1"> </xs:element> diff --git a/daisy-integration/src/main/java/se/su/dsv/scipro/integration/daisy/workers/ProjectExporter.java b/daisy-integration/src/main/java/se/su/dsv/scipro/integration/daisy/workers/ProjectExporter.java index d144f13d40..d6e6ab1c55 100644 --- a/daisy-integration/src/main/java/se/su/dsv/scipro/integration/daisy/workers/ProjectExporter.java +++ b/daisy-integration/src/main/java/se/su/dsv/scipro/integration/daisy/workers/ProjectExporter.java @@ -4,10 +4,9 @@ import static com.querydsl.core.types.dsl.Expressions.anyOf; import jakarta.inject.Inject; import jakarta.inject.Named; -import jakarta.ws.rs.core.Response; import java.time.Instant; +import java.time.LocalDate; import java.time.Period; -import java.util.Calendar; import java.util.Date; import java.util.Optional; import java.util.Set; @@ -38,7 +37,6 @@ import se.su.dsv.scipro.project.Project; import se.su.dsv.scipro.project.ProjectRepo; import se.su.dsv.scipro.project.ProjectStatus; import se.su.dsv.scipro.project.QProject; -import se.su.dsv.scipro.reusable.SciProUtilities; import se.su.dsv.scipro.system.DegreeType; import se.su.dsv.scipro.system.User; import se.su.dsv.scipro.workerthreads.AbstractWorker; @@ -198,7 +196,7 @@ public class ProjectExporter extends AbstractWorker { UnitWithID unit = new UnitWithID(); unit.setId(project.getHeadSupervisor().getUnit().getIdentifier()); - Calendar startDate = SciProUtilities.toCalendar(project.getStartDate()); + LocalDate startDate = project.getStartDate(); String title = finalThesis .map(FinalThesis::getSwedishTitle) @@ -211,7 +209,7 @@ public class ProjectExporter extends AbstractWorker { thesis.setUnit(unit); thesis.setAborted(project.getProjectStatus() == ProjectStatus.INACTIVE); thesis.setStartDate(startDate); - thesis.setEndDate(SciProUtilities.toCalendar(project.getExpectedEndDate())); + thesis.setEndDate(project.getExpectedEndDate()); final ResearchAreas researchAreas = new ResearchAreas(); if (project.getResearchArea() != null && project.getResearchArea().getIdentifier() != null) { final ResearchAreaWithID researchArea2 = new ResearchAreaWithID(); @@ -260,36 +258,14 @@ public class ProjectExporter extends AbstractWorker { } private void updateHeadSupervisor(Project project) throws ExternalExportException { - boolean toAdd = true; + boolean shouldChange = true; Set<ProjectParticipant> contributors = daisyAPI.getContributors(project.getIdentifier()); for (ProjectParticipant contributor : contributors) { if (contributor.getRole() == Role.SUPERVISOR) { - if (contributor.getPerson().getId().equals(project.getHeadSupervisor().getIdentifier())) { - toAdd = false; - } else { - Response response = daisyAPI.deleteThesisPerson( - project.getIdentifier(), - contributor.getPerson().getId() - ); - if (response.getStatusInfo().getFamily() != Response.Status.Family.SUCCESSFUL) { - LOG.warn( - "Failed to delete supervisor from project: {} (id: {} / {})", - project.getTitle(), - project.getId(), - project.getIdentifier() - ); - - // Card 3413 - // Do not attempt to add the new supervisor if the old one is still there to prevent - // the project from having multiple supervisors. - // Hopefully Daisy API will soon have support for the function "change supervisor" - // rather than just INSERT and DELETE rows in a table. - toAdd = false; - } - } + shouldChange = !contributor.getPerson().getId().equals(project.getHeadSupervisor().getIdentifier()); } } - if (toAdd) { + if (shouldChange) { externalExporter.addContributorToProject(project, project.getHeadSupervisor(), Role.SUPERVISOR); } } diff --git a/daisy-integration/src/main/java/se/su/dsv/scipro/io/impl/ExternalExporterDaisyImpl.java b/daisy-integration/src/main/java/se/su/dsv/scipro/io/impl/ExternalExporterDaisyImpl.java index 7ffa26b12e..a8b7184112 100755 --- a/daisy-integration/src/main/java/se/su/dsv/scipro/io/impl/ExternalExporterDaisyImpl.java +++ b/daisy-integration/src/main/java/se/su/dsv/scipro/io/impl/ExternalExporterDaisyImpl.java @@ -3,7 +3,7 @@ package se.su.dsv.scipro.io.impl; import jakarta.inject.Inject; import jakarta.ws.rs.core.Response; import java.math.BigDecimal; -import java.util.Calendar; +import java.time.LocalDate; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import se.su.dsv.scipro.daisyexternal.http.DaisyAPI; @@ -21,7 +21,6 @@ import se.su.dsv.scipro.io.dto.ThesisToBeCreated; import se.su.dsv.scipro.io.dto.UnitWithID; import se.su.dsv.scipro.io.exceptions.ExternalExportException; import se.su.dsv.scipro.project.Project; -import se.su.dsv.scipro.reusable.SciProUtilities; import se.su.dsv.scipro.system.Unit; import se.su.dsv.scipro.system.User; @@ -46,9 +45,9 @@ public class ExternalExporterDaisyImpl implements ExternalExporter { ThesisToBeCreated exportProjectDTO = new ThesisToBeCreated(); exportProjectDTO.setTitle(truncate(project.getTitle())); - Calendar startDate = SciProUtilities.toCalendar(project.getStartDate()); + LocalDate startDate = project.getStartDate(); exportProjectDTO.setStartDate(startDate); - exportProjectDTO.setEndDate(SciProUtilities.toCalendar(project.getExpectedEndDate())); + exportProjectDTO.setEndDate(project.getExpectedEndDate()); exportProjectDTO.setUnit(unitDTO); final ResearchAreas researchAreas = new ResearchAreas(); if (project.getResearchArea() != null && project.getResearchArea().getIdentifier() != null) { diff --git a/daisy-integration/src/test/java/se/su/dsv/scipro/integration/daisy/workers/ProjectExporterTest.java b/daisy-integration/src/test/java/se/su/dsv/scipro/integration/daisy/workers/ProjectExporterTest.java index fec03c6a38..33135eb7f5 100644 --- a/daisy-integration/src/test/java/se/su/dsv/scipro/integration/daisy/workers/ProjectExporterTest.java +++ b/daisy-integration/src/test/java/se/su/dsv/scipro/integration/daisy/workers/ProjectExporterTest.java @@ -2,6 +2,7 @@ package se.su.dsv.scipro.integration.daisy.workers; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -144,22 +145,61 @@ public class ProjectExporterTest { } @Test - public void deletes_wrong_head_supervisor() { - Person person = new Person(); - person.setId(234234); + public void change_head_supervisor() throws Exception { + User newSupervisor = User.builder() + .firstName("New") + .lastName("Supervisor") + .emailAddress("new@supervisor.com") + .build(); + Unit unit = new Unit(); + unit.setIdentifier(239478); - ProjectParticipant headSupervisor = new ProjectParticipant(); - headSupervisor.setRole(Role.SUPERVISOR); - headSupervisor.setPerson(person); + newSupervisor.setIdentifier(12345); + newSupervisor.setUnit(unit); + exportedProject.setHeadSupervisor(newSupervisor); Set<ProjectParticipant> contributors = new HashSet<>(); - contributors.add(headSupervisor); + ProjectParticipant oldSupervisor = new ProjectParticipant(); + oldSupervisor.setRole(Role.SUPERVISOR); + Person oldPerson = new Person(); + oldPerson.setId(SUPERVISOR_IDENTIFIER); + oldSupervisor.setPerson(oldPerson); + contributors.add(oldSupervisor); when(daisyAPI.getContributors(exportedProject.getIdentifier())).thenReturn(contributors); projectExporter.run(); - verify(daisyAPI).deleteThesisPerson(exportedProject.getIdentifier(), person.getId()); + verify(externalExporter).addContributorToProject(exportedProject, newSupervisor, Role.SUPERVISOR); + } + + @Test + public void should_not_change_head_supervisor_if_already_exists() throws Exception { + User newSupervisor = User.builder() + .firstName("New") + .lastName("Supervisor") + .emailAddress("new@supervisor.com") + .build(); + Unit unit = new Unit(); + unit.setIdentifier(239478); + + newSupervisor.setIdentifier(SUPERVISOR_IDENTIFIER); + newSupervisor.setUnit(unit); + exportedProject.setHeadSupervisor(newSupervisor); + + Set<ProjectParticipant> contributors = new HashSet<>(); + ProjectParticipant oldSupervisor = new ProjectParticipant(); + oldSupervisor.setRole(Role.SUPERVISOR); + Person oldPerson = new Person(); + oldPerson.setId(SUPERVISOR_IDENTIFIER); + oldSupervisor.setPerson(oldPerson); + contributors.add(oldSupervisor); + + when(daisyAPI.getContributors(exportedProject.getIdentifier())).thenReturn(contributors); + + projectExporter.run(); + + verify(externalExporter, never()).addContributorToProject(exportedProject, newSupervisor, Role.SUPERVISOR); } @Test diff --git a/daisy-integration/src/test/java/se/su/dsv/scipro/io/impl/ExternalExporterDaisyImplTest.java b/daisy-integration/src/test/java/se/su/dsv/scipro/io/impl/ExternalExporterDaisyImplTest.java index e9c044dabe..c6be049575 100644 --- a/daisy-integration/src/test/java/se/su/dsv/scipro/io/impl/ExternalExporterDaisyImplTest.java +++ b/daisy-integration/src/test/java/se/su/dsv/scipro/io/impl/ExternalExporterDaisyImplTest.java @@ -19,7 +19,6 @@ import org.mockito.junit.jupiter.MockitoExtension; import se.su.dsv.scipro.daisyexternal.http.DaisyAPI; import se.su.dsv.scipro.io.dto.ThesisToBeCreated; import se.su.dsv.scipro.project.Project; -import se.su.dsv.scipro.reusable.SciProUtilities; import se.su.dsv.scipro.system.DegreeType; import se.su.dsv.scipro.system.ProjectType; import se.su.dsv.scipro.system.Unit; @@ -71,6 +70,6 @@ public class ExternalExporterDaisyImplTest { ArgumentCaptor<ThesisToBeCreated> captor = ArgumentCaptor.forClass(ThesisToBeCreated.class); verify(daisyAPI).createProject(captor.capture()); - assertEquals(captor.getValue().getStartDate(), SciProUtilities.toCalendar(daisyStartDate)); + assertEquals(captor.getValue().getStartDate(), daisyStartDate); } }