From 857f6466780b98e5af8dfe9b66253683c5f1629e Mon Sep 17 00:00:00 2001
From: Andreas Svanberg <andreass@dsv.su.se>
Date: Tue, 3 Dec 2024 10:55:28 +0100
Subject: [PATCH 1/7] Upgrade Spring Boot version to address many security
vulnerabilities (#52)
Fixes #28 ([CVE-2024-38809](https://spring.io/security/cve-2024-38809)), #29 ([CVE-2024-38816](https://spring.io/security/cve-2024-38816)), and #46 ([CVE-2024-38820](https://spring.io/security/cve-2024-38820))
Chose to stay on the 3.2 Spring Boot train despite 3.4 being out. Waiting for a more conscious to do the upgrade in case there are other changes required.
Luckily none of the preconditions of the vulnerabilities were true for SciPro so they could not be exploited.
Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/52
Reviewed-by: Tom Zhao <tom.zhao@dsv.su.se>
Co-authored-by: Andreas Svanberg <andreass@dsv.su.se>
Co-committed-by: Andreas Svanberg <andreass@dsv.su.se>
---
pom.xml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/pom.xml b/pom.xml
index e69b87c989..8448e670cf 100755
--- a/pom.xml
+++ b/pom.xml
@@ -101,7 +101,7 @@
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-dependencies</artifactId>
- <version>3.2.5</version>
+ <version>3.2.12</version>
<scope>import</scope>
<type>pom</type>
</dependency>
From c6bd17d9ad5124a9cb334701a181bc4a06a83653 Mon Sep 17 00:00:00 2001
From: Andreas Svanberg <andreass@dsv.su.se>
Date: Mon, 16 Dec 2024 11:24:33 +0100
Subject: [PATCH 2/7] Fix grade calculator being serialized (#59)
The new calculator that's based on templates has a reference to the @Entity for the template which should not be serialized.
Fixes #40
## How to test/replicate
1. Log in as a supervisor
1. Open a project that's new enough to use a grading report template with grade limits
1. Go to the "Finishing up" tab
1. Go to the sub-tab for an individual author
Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/59
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>
---
.../su/dsv/scipro/report/GradeCalculator.java | 4 +---
.../grading/GradingReportPointsPanel.java | 20 ++++++-------------
.../IndividualAuthorAssessmentPanel.java | 4 ++--
.../grading/GradingReportPointsPanelTest.java | 2 +-
4 files changed, 10 insertions(+), 20 deletions(-)
diff --git a/core/src/main/java/se/su/dsv/scipro/report/GradeCalculator.java b/core/src/main/java/se/su/dsv/scipro/report/GradeCalculator.java
index 55cd0c3343..67abd527b3 100644
--- a/core/src/main/java/se/su/dsv/scipro/report/GradeCalculator.java
+++ b/core/src/main/java/se/su/dsv/scipro/report/GradeCalculator.java
@@ -1,8 +1,6 @@
package se.su.dsv.scipro.report;
-import java.io.Serializable;
-
-public interface GradeCalculator extends Serializable {
+public interface GradeCalculator {
GradingReport.Grade getGrade(GradingReport gradingReport);
long getPoints(GradingReport gradingReport);
diff --git a/view/src/main/java/se/su/dsv/scipro/grading/GradingReportPointsPanel.java b/view/src/main/java/se/su/dsv/scipro/grading/GradingReportPointsPanel.java
index 593ee532c5..4f95d80881 100644
--- a/view/src/main/java/se/su/dsv/scipro/grading/GradingReportPointsPanel.java
+++ b/view/src/main/java/se/su/dsv/scipro/grading/GradingReportPointsPanel.java
@@ -4,7 +4,6 @@ import org.apache.wicket.markup.html.WebMarkupContainer;
import org.apache.wicket.markup.html.basic.Label;
import org.apache.wicket.markup.html.panel.Panel;
import org.apache.wicket.model.IModel;
-import org.apache.wicket.model.LoadableDetachableModel;
import se.su.dsv.scipro.components.OppositeVisibility;
import se.su.dsv.scipro.report.GradeCalculator;
import se.su.dsv.scipro.report.GradingReport;
@@ -18,15 +17,13 @@ public class GradingReportPointsPanel extends Panel {
public GradingReportPointsPanel(
String id,
final IModel<? extends GradingReport> gradingReportIModel,
- final GradeCalculator gradeCalculator
+ final IModel<GradeCalculator> gradeCalculator
) {
super(id, gradingReportIModel);
- final IModel<GradingReport.Grade> gradeModel = new LoadableDetachableModel<>() {
- @Override
- protected GradingReport.Grade load() {
- return gradingReportIModel.getObject().getGrade(gradeCalculator);
- }
- };
+ final IModel<GradingReport.Grade> gradeModel = gradingReportIModel.combineWith(
+ gradeCalculator,
+ GradingReport::getGrade
+ );
final Label grade = new Label(GRADE, gradeModel.map(GradingReport.Grade::name)) {
@Override
protected void onConfigure() {
@@ -36,12 +33,7 @@ public class GradingReportPointsPanel extends Panel {
};
add(grade);
- final IModel<Long> points = new LoadableDetachableModel<>() {
- @Override
- protected Long load() {
- return gradingReportIModel.getObject().getPoints(gradeCalculator);
- }
- };
+ final IModel<Long> points = gradingReportIModel.combineWith(gradeCalculator, GradingReport::getPoints);
add(new Label(POINTS_LABEL, points));
add(new WebMarkupContainer(NO_GRADE_EXPLANATION).add(new OppositeVisibility(grade)));
diff --git a/view/src/main/java/se/su/dsv/scipro/grading/IndividualAuthorAssessmentPanel.java b/view/src/main/java/se/su/dsv/scipro/grading/IndividualAuthorAssessmentPanel.java
index 6d4ba8e0d6..c9c0938a58 100644
--- a/view/src/main/java/se/su/dsv/scipro/grading/IndividualAuthorAssessmentPanel.java
+++ b/view/src/main/java/se/su/dsv/scipro/grading/IndividualAuthorAssessmentPanel.java
@@ -271,8 +271,8 @@ public class IndividualAuthorAssessmentPanel extends GenericPanel<User> {
new TemplatePanel("points_to_grade_conversion", gradingReport.map(SupervisorGradingReport::getProject))
);
- GradeCalculator supervisorCalculator = gradeCalculatorService.getSupervisorCalculator(
- gradingReport.getObject().getProject()
+ IModel<GradeCalculator> supervisorCalculator = LoadableDetachableModel.of(() ->
+ gradeCalculatorService.getSupervisorCalculator(gradingReport.getObject().getProject())
);
add(new GradingReportPointsPanel("points", gradingReport, supervisorCalculator));
diff --git a/view/src/test/java/se/su/dsv/scipro/grading/GradingReportPointsPanelTest.java b/view/src/test/java/se/su/dsv/scipro/grading/GradingReportPointsPanelTest.java
index caa5835e3f..f358d419e6 100644
--- a/view/src/test/java/se/su/dsv/scipro/grading/GradingReportPointsPanelTest.java
+++ b/view/src/test/java/se/su/dsv/scipro/grading/GradingReportPointsPanelTest.java
@@ -62,7 +62,7 @@ public class GradingReportPointsPanelTest extends SciProTest {
private void startPanel() {
panel = tester.startComponentInPage(
- new GradingReportPointsPanel("id", Model.of(gradingReport), gradeCalculator)
+ new GradingReportPointsPanel("id", Model.of(gradingReport), () -> gradeCalculator)
);
}
From f67f37ecdd6bf4f45b36e9fb30039f6ad4b7f142 Mon Sep 17 00:00:00 2001
From: Andreas Svanberg <andreass@dsv.su.se>
Date: Mon, 16 Dec 2024 13:23:37 +0100
Subject: [PATCH 3/7] Keep and validate project type selection when
creating/editing application periods (#47)
If you have FormComponents in a ListView you need to call setReuseItems(true) on the ListView. Otherwise the ListItems will be recreated before rendering which results in them losing their "converted input" (what Wicket calls the submitted value).
Instead of simply calling setReuseItems(true) on the ListView, which would've solved the problem, it was instead replaced by a proper FormComponent for dealing with this exact case (a CheckboxMultipleChoice component). This reduces the amount of code required and more clearly communicates intent. The change required some minor test refactoring.
Fixes #33
---
Now requires at least one project type to be selected before saving.
Fixes #34
---
## How to test
1. Go to "Admin" / "Match" / "Application periods"
2. Click create new
3. Submit without selecting any types
4. See that there's proper feedback
5. Leave name blank and select some types
6. Submit
7. See that the project type selection sticks around
Co-authored-by: Nico Athanassiadis <nico@dsv.su.se>
Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/47
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>
---
.../AdminEditApplicationPeriodPage.html | 6 +--
.../AdminEditApplicationPeriodPage.java | 50 ++++---------------
.../AdminEditApplicationPeriodPage.properties | 3 +-
.../AdminEditApplicationPeriodPageTest.java | 25 +++++++++-
4 files changed, 36 insertions(+), 48 deletions(-)
diff --git a/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.html b/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.html
index 04de12622c..8c5597de96 100644
--- a/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.html
+++ b/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.html
@@ -16,10 +16,8 @@
<fieldset class="mb-3">
<legend><wicket:message key="projectTypes"/></legend>
- <div class="form-check" wicket:id="projectTypes">
- <input type="checkbox" wicket:id="checkbox" class="form-check-input"/>
- <label class="form-check-label" wicket:for="checkbox"><span wicket:id="name"></span></label>
- </div>
+ <div wicket:id="projectTypes"></div>
+ <div wicket:id="projectTypesFeedback"></div>
</fieldset>
<div class="form-row">
diff --git a/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.java b/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.java
index 2409d13d83..8781578a73 100644
--- a/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.java
+++ b/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.java
@@ -4,17 +4,14 @@ import jakarta.inject.Inject;
import java.time.LocalDate;
import java.time.LocalTime;
import java.util.List;
-import org.apache.wicket.extensions.model.AbstractCheckBoxModel;
import org.apache.wicket.feedback.FencedFeedbackPanel;
-import org.apache.wicket.markup.html.basic.Label;
import org.apache.wicket.markup.html.form.*;
-import org.apache.wicket.markup.html.list.ListItem;
-import org.apache.wicket.markup.html.list.ListView;
import org.apache.wicket.markup.html.panel.ComponentFeedbackPanel;
import org.apache.wicket.model.IModel;
import org.apache.wicket.model.LambdaModel;
import org.apache.wicket.model.LoadableDetachableModel;
import org.apache.wicket.request.mapper.parameter.PageParameters;
+import se.su.dsv.scipro.components.BootstrapCheckBoxMultipleChoice;
import se.su.dsv.scipro.components.BootstrapDatePicker;
import se.su.dsv.scipro.components.BootstrapTimePicker;
import se.su.dsv.scipro.components.DatesValidator;
@@ -39,7 +36,6 @@ public class AdminEditApplicationPeriodPage
public static final String TITLE = "title";
public static final String FEEDBACK = "Feedback";
public static final String TITLE_FEEDBACK = "titleFeedback";
- public static final String CHECKBOX = "checkbox";
@Inject
private ProjectTypeService projectTypeService;
@@ -62,19 +58,15 @@ public class AdminEditApplicationPeriodPage
);
add(new ComponentFeedbackPanel(TITLE_FEEDBACK, title));
add(title);
- add(
- new ListView<>(PROJECT_TYPES, availableProjectTypes()) {
- @Override
- protected void populateItem(ListItem<ProjectType> item) {
- item.add(
- new CheckBox(CHECKBOX, new ProjectTypeSelectionModel(item.getModel())).setOutputMarkupId(
- true
- )
- );
- item.add(new Label("name", item.getModel().map(ProjectType::getName)));
- }
- }
+ BootstrapCheckBoxMultipleChoice<ProjectType> projectTypeChoice = new BootstrapCheckBoxMultipleChoice<>(
+ PROJECT_TYPES,
+ LambdaModel.of(getModel(), ApplicationPeriod::getProjectTypes, ApplicationPeriod::setProjectTypes),
+ availableProjectTypes(),
+ new LambdaChoiceRenderer<>(ProjectType::getName, ProjectType::getId)
);
+ projectTypeChoice.setRequired(true);
+ add(projectTypeChoice);
+ add(new FencedFeedbackPanel("projectTypesFeedback", projectTypeChoice));
final FormComponent<LocalDate> startDate = addDateField(
START_DATE,
LambdaModel.of(getModel(), ApplicationPeriod::getStartDate, ApplicationPeriod::setStartDate)
@@ -139,30 +131,6 @@ public class AdminEditApplicationPeriodPage
getRootForm().error(getString("overlapping"));
}
}
-
- private class ProjectTypeSelectionModel extends AbstractCheckBoxModel {
-
- private final IModel<ProjectType> model;
-
- public ProjectTypeSelectionModel(IModel<ProjectType> model) {
- this.model = model;
- }
-
- @Override
- public boolean isSelected() {
- return getModelObject().getProjectTypes().contains(model.getObject());
- }
-
- @Override
- public void select() {
- getModelObject().addProjectType(model.getObject());
- }
-
- @Override
- public void unselect() {
- getModelObject().removeProjectType(model.getObject());
- }
- }
}
private LoadableDetachableModel<ApplicationPeriod> getLoaded(final PageParameters pp) {
diff --git a/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.properties b/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.properties
index f5095052d4..dc945a5788 100644
--- a/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.properties
+++ b/view/src/main/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPage.properties
@@ -14,4 +14,5 @@ success= Application period saved.
overlapping= Overlapping application period already exists.
date.Required= You need to specify a valid date.
hours.Required= Hours field is required.
-minutes.Required= Minutes field is required.
\ No newline at end of file
+minutes.Required= Minutes field is required.
+projectTypes.Required=You must select at least one project type.
diff --git a/view/src/test/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPageTest.java b/view/src/test/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPageTest.java
index 7b68730909..a6bc599bd9 100644
--- a/view/src/test/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPageTest.java
+++ b/view/src/test/java/se/su/dsv/scipro/applicationperiod/AdminEditApplicationPeriodPageTest.java
@@ -7,8 +7,10 @@ import java.io.Serializable;
import java.time.LocalDate;
import java.util.Collections;
import java.util.List;
+import org.apache.wicket.Component;
import org.apache.wicket.Page;
import org.apache.wicket.feedback.FeedbackMessage;
+import org.apache.wicket.markup.html.form.CheckBoxMultipleChoice;
import org.apache.wicket.markup.html.form.RequiredTextField;
import org.apache.wicket.request.mapper.parameter.PageParameters;
import org.apache.wicket.util.tester.FormTester;
@@ -36,6 +38,7 @@ public class AdminEditApplicationPeriodPageTest extends SciProTest {
@BeforeEach
public void setUp() throws Exception {
bachelor = new ProjectType(DegreeType.BACHELOR, "Bachelor", "Bachelor");
+ bachelor.setId(8L);
bachelor.addModule(ProjectModule.MATCH);
when(projectTypeService.findWithModule(ProjectModule.MATCH)).thenReturn(Collections.singletonList(bachelor));
startPage();
@@ -47,8 +50,12 @@ public class AdminEditApplicationPeriodPageTest extends SciProTest {
}
@Test
+ @SuppressWarnings("unchecked")
public void contains_project_type_selection() {
- tester.assertModelValue(path(FORM, PROJECT_TYPES), projectTypeService.findWithModule(ProjectModule.MATCH));
+ tester.assertComponent(path(FORM, PROJECT_TYPES), CheckBoxMultipleChoice.class);
+ Component component = tester.getComponentFromLastRenderedPage(path(FORM, PROJECT_TYPES));
+ CheckBoxMultipleChoice<ProjectType> choice = (CheckBoxMultipleChoice<ProjectType>) component;
+ Assertions.assertEquals(projectTypeService.findWithModule(ProjectModule.MATCH), choice.getChoices());
}
@Test
@@ -105,7 +112,7 @@ public class AdminEditApplicationPeriodPageTest extends SciProTest {
);
FormTester formTester = tester.newFormTester(FORM);
fillInForm("Title", 0, 1, 2, formTester);
- formTester.setValue(path(PROJECT_TYPES, 0, CHECKBOX), true);
+ formTester.setValue(path(PROJECT_TYPES), String.valueOf(bachelor.getId()));
formTester.submit();
ArgumentCaptor<ApplicationPeriod> captor = ArgumentCaptor.forClass(ApplicationPeriod.class);
@@ -113,12 +120,25 @@ public class AdminEditApplicationPeriodPageTest extends SciProTest {
MatcherAssert.assertThat(captor.getValue().getProjectTypes(), Matchers.hasItem(bachelor));
}
+ @Test
+ public void requires_at_least_one_project_type_to_be_selected() {
+ FormTester formTester = tester.newFormTester(FORM);
+ fillInFormWithValidValues(formTester);
+ formTester.setValue(path(PROJECT_TYPES), "");
+ formTester.submit();
+ tester.assertErrorMessages(tester.getLastRenderedPage().getString("projectTypes.Required"));
+ }
+
private void submitForm(String title, int startDate, int endDate, int courseStartDate) {
FormTester formTester = tester.newFormTester(FORM);
fillInForm(title, startDate, endDate, courseStartDate, formTester);
formTester.submit();
}
+ private void fillInFormWithValidValues(FormTester formTester) {
+ fillInForm("Title", 0, 1, 2, formTester);
+ }
+
private void fillInForm(String title, int startDate, int endDate, int courseStartDate, FormTester formTester) {
formTester.setValue(TITLE, title);
final LocalDate now = LocalDate.now();
@@ -126,6 +146,7 @@ public class AdminEditApplicationPeriodPageTest extends SciProTest {
formTester.setValue(END_DATE, now.plusDays(endDate).toString());
formTester.setValue(COURSE_START_DATE, now.plusDays(courseStartDate).toString());
formTester.setValue("courseStartTime", "08:00");
+ formTester.setValue(PROJECT_TYPES, String.valueOf(bachelor.getId()));
}
private void startPage() {
From 89c8a4f8a25c43376e5f1eea6f80be7c8505fa33 Mon Sep 17 00:00:00 2001
From: Andreas Svanberg <andreass@dsv.su.se>
Date: Mon, 16 Dec 2024 13:26:19 +0100
Subject: [PATCH 4/7] Update instructions for how to get Prettier to format on
save (#55)
IntelliJ requires Node.js to be installed for it to be able to run Prettier and format the code.
Co-authored-by: Nico Athanassiadis <nico@dsv.su.se>
Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/55
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>
---
README.md | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/README.md b/README.md
index dc2b0cb977..f5b81e7a88 100644
--- a/README.md
+++ b/README.md
@@ -19,9 +19,14 @@ to format all Java code. To reformat the code run
Yes it's a mouthful but unfortunately the [prettier-maven-plugin](https://github.com/HubSpot/prettier-maven-plugin)
does not work due to an [outstanding issue](https://github.com/HubSpot/prettier-maven-plugin/issues/79).
-An easier way to reformat code is to set IntelliJ to do it on save. Go to
-`Settings -> Language & Frameworks -> JavaScript -> Prettier` and then check
+The formatting is validated by CI, but you should do it beforehand with a simple `./mvnw verify -pl .`.
+
+### Making IntelliJ format for you
+For this to work you also need to have [Node.js](https://nodejs.org)
+installed and configured under `Settings -> Language & Frameworks -> Node.js`
+and the file you're saving *must* be able to compile otherwise no formatting
+can be performed.
+
+Go to `Settings -> Language & Frameworks -> JavaScript -> Prettier` and then check
`Automatic Prettier Configuration`, set `Run for files` to `**/*.{java}`,
and finally check `Run on save`.
-
-The formatting is validated by CI, but you should do it beforehand with a simple `./mvnw verify -pl .`.
From 5fbf4ec0c0591dd42a7ce7101096b5eba1c36599 Mon Sep 17 00:00:00 2001
From: Andreas Svanberg <andreass@dsv.su.se>
Date: Mon, 16 Dec 2024 13:55:33 +0100
Subject: [PATCH 5/7] Switch from an in-memory HSQLDB to MariaDB during
integration tests (#57)
Currently our integration tests run against an in-memory HSQLDB whose schema is created by Hibernate based on our JPA annotations. This has differences from the MariaDB schema created by our Flyway migrations. It is also a completely different database engine so who knows what other differences there are.
This proposal changes this so that it will instead use [Testcontainers](https://testcontainers.com/) to spin up a MariaDB Docker container that then has the Flyway migrations ran before being used in tests.
Pros:
* Same database engine in tests as production
* Flyway migrations are tested
* Database schema is the same in tests as production (`NOT NULL` constraints, foreign keys, and so on)
Cons:
* *Much* slower test executions and they will get slower over time as more migrations are added
Co-authored-by: Nico Athanassiadis <nico@dsv.su.se>
Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/57
Reviewed-by: Nico Athanassiadis <nico@dsv.su.se>
---
core/pom.xml | 19 +++++++++++++--
.../main/resources/META-INF/persistence.xml | 12 ----------
.../FinalSeminarSchedulingTest.java | 1 +
.../peer/CommentThreadServiceImplTest.java | 2 ++
.../peer/PeerRequestServiceImplTest.java | 2 ++
.../PeerReviewServiceImplIntegrationTest.java | 2 ++
.../peer/PeerReviewServiceImplTest.java | 2 ++
.../se/su/dsv/scipro/peer/TestPeerReview.java | 2 ++
.../UrkundSubmissionRepositoryTest.java | 4 ++++
.../se/su/dsv/scipro/test/SpringTest.java | 23 +++++++++++++++++--
pom.xml | 9 --------
11 files changed, 53 insertions(+), 25 deletions(-)
diff --git a/core/pom.xml b/core/pom.xml
index a3c2a1a1e9..e34cb44596 100644
--- a/core/pom.xml
+++ b/core/pom.xml
@@ -86,8 +86,23 @@
<scope>test</scope>
</dependency>
<dependency>
- <groupId>org.hsqldb</groupId>
- <artifactId>hsqldb</artifactId>
+ <groupId>org.testcontainers</groupId>
+ <artifactId>junit-jupiter</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.testcontainers</groupId>
+ <artifactId>mariadb</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.mariadb.jdbc</groupId>
+ <artifactId>mariadb-java-client</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.flywaydb</groupId>
+ <artifactId>flyway-mysql</artifactId>
<scope>test</scope>
</dependency>
diff --git a/core/src/main/resources/META-INF/persistence.xml b/core/src/main/resources/META-INF/persistence.xml
index fc5f0a5977..902cc7aeae 100755
--- a/core/src/main/resources/META-INF/persistence.xml
+++ b/core/src/main/resources/META-INF/persistence.xml
@@ -4,9 +4,6 @@
xsi:schemaLocation="https://jakarta.ee/xml/ns/persistence https://jakarta.ee/xml/ns/persistence/persistence_3_0.xsd"
version="3.0">
- <!-- NOTE THAT THERE ARE TWO PERSISTENCE UNITS, one default and one test
- used for either running or unit-tests -->
-
<!-- A JPA Persistence Unit -->
<persistence-unit name="defaultPersistenceUnit"
transaction-type="RESOURCE_LOCAL">
@@ -17,13 +14,4 @@
</properties>
</persistence-unit>
- <!-- A JPA Persistence Unit used for tests -->
- <persistence-unit name="testPersistenceUnit"
- transaction-type="RESOURCE_LOCAL">
- <properties>
- <property name="jakarta.persistence.jdbc.driver" value="org.hsqldb.jdbc.JDBCDriver"/>
- <property name="jakarta.persistence.jdbc.url" value="jdbc:hsqldb:mem:test"/>
- <property name="hibernate.hbm2ddl.auto" value="create"/>
- </properties>
- </persistence-unit>
</persistence>
diff --git a/core/src/test/java/se/su/dsv/scipro/finalseminar/FinalSeminarSchedulingTest.java b/core/src/test/java/se/su/dsv/scipro/finalseminar/FinalSeminarSchedulingTest.java
index 23ebd8113c..cca047aed5 100644
--- a/core/src/test/java/se/su/dsv/scipro/finalseminar/FinalSeminarSchedulingTest.java
+++ b/core/src/test/java/se/su/dsv/scipro/finalseminar/FinalSeminarSchedulingTest.java
@@ -197,6 +197,7 @@ public class FinalSeminarSchedulingTest extends IntegrationTest {
NonWorkDayPeriod nonWorkDayPeriod = new NonWorkDayPeriod();
nonWorkDayPeriod.setStartDate(date);
nonWorkDayPeriod.setEndDate(date);
+ nonWorkDayPeriod.setComment("test non work day");
save(nonWorkDayPeriod);
}
diff --git a/core/src/test/java/se/su/dsv/scipro/peer/CommentThreadServiceImplTest.java b/core/src/test/java/se/su/dsv/scipro/peer/CommentThreadServiceImplTest.java
index cd037f3ee5..625162f167 100644
--- a/core/src/test/java/se/su/dsv/scipro/peer/CommentThreadServiceImplTest.java
+++ b/core/src/test/java/se/su/dsv/scipro/peer/CommentThreadServiceImplTest.java
@@ -12,6 +12,7 @@ import se.su.dsv.scipro.file.FileDescription;
import se.su.dsv.scipro.file.FileReference;
import se.su.dsv.scipro.project.Project;
import se.su.dsv.scipro.system.DegreeType;
+import se.su.dsv.scipro.system.Language;
import se.su.dsv.scipro.system.ProjectType;
import se.su.dsv.scipro.system.User;
import se.su.dsv.scipro.test.Dates;
@@ -66,6 +67,7 @@ public class CommentThreadServiceImplTest extends IntegrationTest {
PeerRequest peerRequest = new PeerRequest();
peerRequest.setProject(project);
peerRequest.setRequester(createUser());
+ peerRequest.setLanguage(Language.ENGLISH);
final FileDescription fileDescription = save(new FileDescription());
final FileReference fileReference = new FileReference();
fileReference.setFileDescription(fileDescription);
diff --git a/core/src/test/java/se/su/dsv/scipro/peer/PeerRequestServiceImplTest.java b/core/src/test/java/se/su/dsv/scipro/peer/PeerRequestServiceImplTest.java
index 215e6c5f45..fc98ba6f7b 100644
--- a/core/src/test/java/se/su/dsv/scipro/peer/PeerRequestServiceImplTest.java
+++ b/core/src/test/java/se/su/dsv/scipro/peer/PeerRequestServiceImplTest.java
@@ -13,6 +13,7 @@ import se.su.dsv.scipro.file.FileDescription;
import se.su.dsv.scipro.file.FileReference;
import se.su.dsv.scipro.project.Project;
import se.su.dsv.scipro.system.DegreeType;
+import se.su.dsv.scipro.system.Language;
import se.su.dsv.scipro.system.PageRequest;
import se.su.dsv.scipro.system.ProjectType;
import se.su.dsv.scipro.system.User;
@@ -158,6 +159,7 @@ public class PeerRequestServiceImplTest extends IntegrationTest {
peerRequest.setProject(project);
peerRequest.setRequester(requester);
peerRequest.setStatus(status);
+ peerRequest.setLanguage(Language.ENGLISH);
final FileDescription fileDescription = save(new FileDescription());
final FileReference fileReference = new FileReference();
fileReference.setFileDescription(fileDescription);
diff --git a/core/src/test/java/se/su/dsv/scipro/peer/PeerReviewServiceImplIntegrationTest.java b/core/src/test/java/se/su/dsv/scipro/peer/PeerReviewServiceImplIntegrationTest.java
index 15a004fd89..a954f455be 100644
--- a/core/src/test/java/se/su/dsv/scipro/peer/PeerReviewServiceImplIntegrationTest.java
+++ b/core/src/test/java/se/su/dsv/scipro/peer/PeerReviewServiceImplIntegrationTest.java
@@ -14,6 +14,7 @@ import se.su.dsv.scipro.file.FileDescription;
import se.su.dsv.scipro.file.FileReference;
import se.su.dsv.scipro.project.Project;
import se.su.dsv.scipro.system.DegreeType;
+import se.su.dsv.scipro.system.Language;
import se.su.dsv.scipro.system.PageRequest;
import se.su.dsv.scipro.system.ProjectType;
import se.su.dsv.scipro.system.User;
@@ -102,6 +103,7 @@ public class PeerReviewServiceImplIntegrationTest extends IntegrationTest {
PeerRequest peerRequest = new PeerRequest();
peerRequest.setProject(createProject());
peerRequest.setRequester(createUser());
+ peerRequest.setLanguage(Language.ENGLISH);
final FileDescription fileDescription = save(new FileDescription());
final FileReference fileReference = new FileReference();
fileReference.setFileDescription(fileDescription);
diff --git a/core/src/test/java/se/su/dsv/scipro/peer/PeerReviewServiceImplTest.java b/core/src/test/java/se/su/dsv/scipro/peer/PeerReviewServiceImplTest.java
index 9c6a5dd3da..6495faaf7a 100644
--- a/core/src/test/java/se/su/dsv/scipro/peer/PeerReviewServiceImplTest.java
+++ b/core/src/test/java/se/su/dsv/scipro/peer/PeerReviewServiceImplTest.java
@@ -14,6 +14,7 @@ import se.su.dsv.scipro.file.FileDescription;
import se.su.dsv.scipro.file.FileReference;
import se.su.dsv.scipro.project.Project;
import se.su.dsv.scipro.system.DegreeType;
+import se.su.dsv.scipro.system.Language;
import se.su.dsv.scipro.system.PageRequest;
import se.su.dsv.scipro.system.ProjectType;
import se.su.dsv.scipro.system.User;
@@ -95,6 +96,7 @@ public class PeerReviewServiceImplTest extends IntegrationTest {
peerRequest.setRequester(
save(User.builder().firstName("Bob").lastName("Sponge").emailAddress("bob@example.com").build())
);
+ peerRequest.setLanguage(Language.ENGLISH);
final FileDescription fileDescription = save(new FileDescription());
final FileReference fileReference = new FileReference();
fileReference.setFileDescription(fileDescription);
diff --git a/core/src/test/java/se/su/dsv/scipro/peer/TestPeerReview.java b/core/src/test/java/se/su/dsv/scipro/peer/TestPeerReview.java
index 34e5e5c7e2..fe5e2be992 100755
--- a/core/src/test/java/se/su/dsv/scipro/peer/TestPeerReview.java
+++ b/core/src/test/java/se/su/dsv/scipro/peer/TestPeerReview.java
@@ -19,6 +19,7 @@ import se.su.dsv.scipro.file.FileDescription;
import se.su.dsv.scipro.file.FileReference;
import se.su.dsv.scipro.project.Project;
import se.su.dsv.scipro.project.ProjectStatus;
+import se.su.dsv.scipro.system.Language;
import se.su.dsv.scipro.system.PageRequest;
import se.su.dsv.scipro.system.ProjectType;
import se.su.dsv.scipro.system.User;
@@ -196,6 +197,7 @@ public class TestPeerReview extends IntegrationTest {
request.setComment(comment);
request.setRequester(requester);
request.setProject(project);
+ request.setLanguage(Language.ENGLISH);
final FileDescription fileDescription = save(new FileDescription());
final FileReference fileReference = new FileReference();
fileReference.setFileDescription(fileDescription);
diff --git a/core/src/test/java/se/su/dsv/scipro/plagiarism/urkund/UrkundSubmissionRepositoryTest.java b/core/src/test/java/se/su/dsv/scipro/plagiarism/urkund/UrkundSubmissionRepositoryTest.java
index c09f3ea67e..a3b40ed7ae 100644
--- a/core/src/test/java/se/su/dsv/scipro/plagiarism/urkund/UrkundSubmissionRepositoryTest.java
+++ b/core/src/test/java/se/su/dsv/scipro/plagiarism/urkund/UrkundSubmissionRepositoryTest.java
@@ -13,6 +13,7 @@ import org.hamcrest.TypeSafeMatcher;
import org.junit.jupiter.api.Test;
import se.su.dsv.scipro.file.FileDescription;
import se.su.dsv.scipro.file.FileReference;
+import se.su.dsv.scipro.system.User;
import se.su.dsv.scipro.test.SpringTest;
public class UrkundSubmissionRepositoryTest extends SpringTest {
@@ -23,11 +24,14 @@ public class UrkundSubmissionRepositoryTest extends SpringTest {
@Test
public void save() {
final Instant submitted = Instant.now();
+ User bob = User.builder().firstName("Bob").lastName("Sponge").emailAddress("bob@example.com").build();
+ save(bob);
final UrkundSubmission submission = new UrkundSubmission();
submission.setState(UrkundSubmission.State.SUBMITTED);
submission.setMessage("Hi");
submission.setSubmitted(submitted);
submission.setNextPoll(submitted);
+ submission.setReceiver(bob);
final FileDescription file = save(new FileDescription());
final FileReference fileReference = new FileReference();
fileReference.setFileDescription(file);
diff --git a/core/src/test/java/se/su/dsv/scipro/test/SpringTest.java b/core/src/test/java/se/su/dsv/scipro/test/SpringTest.java
index 2b68b55008..04d0f70da7 100644
--- a/core/src/test/java/se/su/dsv/scipro/test/SpringTest.java
+++ b/core/src/test/java/se/su/dsv/scipro/test/SpringTest.java
@@ -4,28 +4,47 @@ import jakarta.persistence.EntityManager;
import jakarta.persistence.EntityManagerFactory;
import jakarta.persistence.EntityTransaction;
import jakarta.persistence.Persistence;
+import java.sql.SQLException;
import java.time.Clock;
+import java.util.Map;
import java.util.Optional;
+import org.flywaydb.core.Flyway;
+import org.hibernate.cfg.Environment;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
+import org.mariadb.jdbc.MariaDbDataSource;
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Import;
+import org.testcontainers.containers.MariaDBContainer;
+import org.testcontainers.junit.jupiter.Container;
+import org.testcontainers.junit.jupiter.Testcontainers;
import se.su.dsv.scipro.CoreConfig;
import se.su.dsv.scipro.RepositoryConfiguration;
import se.su.dsv.scipro.profiles.CurrentProfile;
import se.su.dsv.scipro.sukat.Sukat;
import se.su.dsv.scipro.system.CurrentUser;
+@Testcontainers
public abstract class SpringTest {
private EntityManager entityManager;
private EntityManagerFactory entityManagerFactory;
+ @Container
+ static MariaDBContainer<?> mariaDBContainer = new MariaDBContainer<>("mariadb:10.11");
+
@BeforeEach
- public final void prepareSpring() {
- entityManagerFactory = Persistence.createEntityManagerFactory("testPersistenceUnit");
+ public final void prepareSpring() throws SQLException {
+ MariaDbDataSource dataSource = new MariaDbDataSource(mariaDBContainer.getJdbcUrl());
+ dataSource.setUser(mariaDBContainer.getUsername());
+ dataSource.setPassword(mariaDBContainer.getPassword());
+
+ Flyway.configure().dataSource(dataSource).load().migrate();
+
+ Map<String, Object> jpaProperties = Map.of(Environment.JAKARTA_JTA_DATASOURCE, dataSource);
+ entityManagerFactory = Persistence.createEntityManagerFactory("defaultPersistenceUnit", jpaProperties);
this.entityManager = entityManagerFactory.createEntityManager();
EntityTransaction transaction = entityManager.getTransaction();
transaction.begin();
diff --git a/pom.xml b/pom.xml
index 8448e670cf..a4b4f57961 100755
--- a/pom.xml
+++ b/pom.xml
@@ -34,7 +34,6 @@
<querydsl.version>5.0.0</querydsl.version>
<jakarta.servlet.version>5.0.0</jakarta.servlet.version>
<junit.version>5.9.3</junit.version>
- <hsqldb.version>2.7.1</hsqldb.version>
<mockito.version>5.3.1</mockito.version>
<flyway.version>9.19.1</flyway.version>
<jersey.version>3.1.6</jersey.version>
@@ -133,14 +132,6 @@
<scope>import</scope>
</dependency>
- <!-- Database stuff -->
- <dependency>
- <groupId>org.hsqldb</groupId>
- <artifactId>hsqldb</artifactId>
- <version>${hsqldb.version}</version>
- <scope>test</scope>
- </dependency>
-
<dependency>
<groupId>org.mariadb.jdbc</groupId>
<artifactId>mariadb-java-client</artifactId>
From a1d3d0be8d813593b8b9a255c0bbc9746b707169 Mon Sep 17 00:00:00 2001
From: Andreas Svanberg <andreass@dsv.su.se>
Date: Mon, 16 Dec 2024 14:20:41 +0100
Subject: [PATCH 6/7] Fix certain milestones not getting activated (#54)
There was a missing bean definition that was responsible for marking certain milestones based on system events.
Fixes #53
## How to test
1. Log in as an author
2. Go to "Peer" tab within a project
3. Request a peer review
4. [If needed] Switch to another author and request another review
5. Perform a peer review
6. See that the peer reviewer completed milestone (first or second depending) is marked as completed
Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/54
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>
---
.../main/java/se/su/dsv/scipro/CoreConfig.java | 18 ++++++++++++++++++
.../dsv/scipro/test/BeanDefinitionsTest.java | 18 ++++++++++++++++++
2 files changed, 36 insertions(+)
create mode 100644 core/src/test/java/se/su/dsv/scipro/test/BeanDefinitionsTest.java
diff --git a/core/src/main/java/se/su/dsv/scipro/CoreConfig.java b/core/src/main/java/se/su/dsv/scipro/CoreConfig.java
index 2dc8f54929..43c4d7ed10 100644
--- a/core/src/main/java/se/su/dsv/scipro/CoreConfig.java
+++ b/core/src/main/java/se/su/dsv/scipro/CoreConfig.java
@@ -96,6 +96,7 @@ import se.su.dsv.scipro.match.TargetRepository;
import se.su.dsv.scipro.match.TargetServiceImpl;
import se.su.dsv.scipro.milestones.MilestoneActivityTemplateRepository;
import se.su.dsv.scipro.milestones.service.ActivateCompletedMilestonesOnNewProjects;
+import se.su.dsv.scipro.milestones.service.MilestoneActivator;
import se.su.dsv.scipro.milestones.service.MilestoneActivityTemplateService;
import se.su.dsv.scipro.milestones.service.impl.MilestoneActivityTemplateServiceImpl;
import se.su.dsv.scipro.milestones.service.impl.MilestonePhaseTemplateServiceImpl;
@@ -1129,4 +1130,21 @@ public class CoreConfig {
public GroupFacadeImpl groupFacade() {
return new GroupFacadeImpl();
}
+
+ @Bean
+ public MilestoneActivator milestoneActivator(
+ EventBus eventBus,
+ MilestoneServiceImpl milestoneService,
+ MilestoneActivityTemplateService milestoneActivityTemplateService,
+ FinalSeminarService finalSeminarService,
+ NotificationController notificationController
+ ) {
+ return new MilestoneActivator(
+ milestoneActivityTemplateService,
+ milestoneService,
+ eventBus,
+ finalSeminarService,
+ notificationController
+ );
+ }
}
diff --git a/core/src/test/java/se/su/dsv/scipro/test/BeanDefinitionsTest.java b/core/src/test/java/se/su/dsv/scipro/test/BeanDefinitionsTest.java
new file mode 100644
index 0000000000..8dd6ff6130
--- /dev/null
+++ b/core/src/test/java/se/su/dsv/scipro/test/BeanDefinitionsTest.java
@@ -0,0 +1,18 @@
+package se.su.dsv.scipro.test;
+
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+import jakarta.inject.Inject;
+import org.junit.jupiter.api.Test;
+import se.su.dsv.scipro.milestones.service.MilestoneActivator;
+
+public class BeanDefinitionsTest extends IntegrationTest {
+
+ @Inject
+ MilestoneActivator milestoneActivator;
+
+ @Test
+ public void milestone_activator() {
+ assertNotNull(milestoneActivator);
+ }
+}
From 25117c8187ebc83f93006d13c3427a7e687e632e Mon Sep 17 00:00:00 2001
From: Andreas Svanberg <andreass@dsv.su.se>
Date: Mon, 16 Dec 2024 16:55:49 +0100
Subject: [PATCH 7/7] Switch authentication to OAuth 2 (#27)
This is one requirement in bringing #15 to reality.
Currently the way to log in to SciPro is by having a locally modified `web.xml` that emulates being authenticated via single sign-on (SSO). This method can not work on an automatically deployed test server. It is also not possible to have real SSO configured for the test servers due to their dynamic nature and that they are given a new hostname each time. Our current SSO solution requires there to be certificate issued to specific hostnames. Even if it were possible to get SSO set up how would the username received from SSO match to test data? We would have to have real usernames in our test data which is not desirable.
To solve both of the problems described above - requiring a locally modified version of a git tracked file and needing an authentication mechanism that works for dynamic test servers - a change of the authentication mechanism from Tomcat controlled SSO to application controlled OAuth 2 is proposed. There is already an OAuth 2 authorization server running in production which itself is authenticates users via SSO that will be used in production and for the permanent test servers. In development and for the dynamic test servers a local authorization server running in Docker is provided.
For "regular" users there will be no noticeable change, they will be prompted to log in via SSO and then they get access to the system. For users with high developer access they will, on the permanent test servers, be prompted to "issue token". On that page they can use the top form to authenticate as themselves based on their SSO authentication, or use the bottom form to issue a completely custom authentication and log in as whatever username they deem necessary. The temporary test servers and during local development will work similarly with the only difference being that there is no SSO log in first and you will be prompted to issue a token immediately. The default authentication (top form) will be a local sys-admin level user.
## How to test
1. Start the local OAuth 2 authorization server with `docker compose up`
2. Start SciPro
3. Attempt to log in
Co-authored-by: Nico Athanassiadis <nico@dsv.su.se>
Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/27
Reviewed-by: Nico Athanassiadis <nico@dsv.su.se>
---
README.md | 9 ++
.../se/su/dsv/scipro/DataInitializer.java | 4 +
.../scipro/system/AuthenticationContext.java | 15 +++
.../impl/ImporterTransactionsImpl.java | 1 +
docker-compose.yml | 11 +++
.../scipro/CurrentUserFromWicketSession.java | 14 ---
.../dsv/scipro/loginlogout/pages/SSOPage.java | 7 +-
.../security/auth/MockRemoteUserFilter.java | 81 ---------------
.../su/dsv/scipro/session/SciProSession.java | 13 +--
.../java/se/su/dsv/scipro/SciProTest.java | 6 +-
war/pom.xml | 4 +
.../war/CurrentUserFromSpringSecurity.java | 98 +++++++++++++++++++
.../main/java/se/su/dsv/scipro/war/Main.java | 6 --
.../dsv/scipro/war/WicketConfiguration.java | 27 +++++
war/src/main/resources/application.properties | 11 +++
15 files changed, 192 insertions(+), 115 deletions(-)
create mode 100644 core/src/main/java/se/su/dsv/scipro/system/AuthenticationContext.java
delete mode 100644 view/src/main/java/se/su/dsv/scipro/CurrentUserFromWicketSession.java
delete mode 100755 view/src/main/java/se/su/dsv/scipro/security/auth/MockRemoteUserFilter.java
create mode 100644 war/src/main/java/se/su/dsv/scipro/war/CurrentUserFromSpringSecurity.java
diff --git a/README.md b/README.md
index f5b81e7a88..22533e79fc 100644
--- a/README.md
+++ b/README.md
@@ -1,3 +1,12 @@
+## Working with the web GUI (Wicket)
+The web GUI is protected by OAuth 2 log in. Run the Docker Compose containers with
+`docker compose up` to start the authorization server to be able to log in.
+
+If you run SciPro in development mode (DEV profile) you will be able to log in
+as the "default" OAuth 2 user populated in the upper form. If you have other
+data in your database you will have to use the lower form and specify a valid
+username in the principal field.
+
## Working with the API
The API is protected by OAuth 2 acting as a [resource server](https://www.oauth.com/oauth2-servers/the-resource-server/)
verifying tokens using [token introspection](https://datatracker.ietf.org/doc/html/rfc7662).
diff --git a/core/src/main/java/se/su/dsv/scipro/DataInitializer.java b/core/src/main/java/se/su/dsv/scipro/DataInitializer.java
index aee6d0b488..7ab805cd7a 100644
--- a/core/src/main/java/se/su/dsv/scipro/DataInitializer.java
+++ b/core/src/main/java/se/su/dsv/scipro/DataInitializer.java
@@ -206,6 +206,10 @@ public class DataInitializer implements Lifecycle {
admin.addRole(Roles.SYSADMIN);
createBeta(admin);
passwordService.updatePassword(admin, "aey7ru8aefei0jaW2wo9eX8EiShi0aan");
+ Username defaultOAuth2Principal = new Username();
+ defaultOAuth2Principal.setUsername("dev@localhost");
+ defaultOAuth2Principal.setUser(admin);
+ save(defaultOAuth2Principal);
}
private void createBeta(User user) {
diff --git a/core/src/main/java/se/su/dsv/scipro/system/AuthenticationContext.java b/core/src/main/java/se/su/dsv/scipro/system/AuthenticationContext.java
new file mode 100644
index 0000000000..9f46604358
--- /dev/null
+++ b/core/src/main/java/se/su/dsv/scipro/system/AuthenticationContext.java
@@ -0,0 +1,15 @@
+package se.su.dsv.scipro.system;
+
+/**
+ * Information about the current authentication context.
+ * <p>
+ * The difference between this and {@link CurrentUser} is that a user can be
+ * authenticated without being a user in the system. This can happen when a
+ * user logs in for the first time via SSO. The {@link #set(User)} method can
+ * be used if the user can be imported based on the {@link #getPrincipalName()}.
+ */
+public interface AuthenticationContext extends CurrentUser {
+ void set(User user);
+
+ String getPrincipalName();
+}
diff --git a/daisy-integration/src/main/java/se/su/dsv/scipro/daisyExternal/impl/ImporterTransactionsImpl.java b/daisy-integration/src/main/java/se/su/dsv/scipro/daisyExternal/impl/ImporterTransactionsImpl.java
index 0eca54eadf..395f5485ef 100644
--- a/daisy-integration/src/main/java/se/su/dsv/scipro/daisyExternal/impl/ImporterTransactionsImpl.java
+++ b/daisy-integration/src/main/java/se/su/dsv/scipro/daisyExternal/impl/ImporterTransactionsImpl.java
@@ -171,6 +171,7 @@ public class ImporterTransactionsImpl implements ImporterTransactions {
username.setUsername(completeUsername);
username.setUser(local);
userNameService.save(username);
+ local.getUsernames().add(username);
}
}
}
diff --git a/docker-compose.yml b/docker-compose.yml
index 637455a39e..aac221e274 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -13,3 +13,14 @@ services:
- CLIENT_REDIRECT_URI=http://localhost:59732/
- RESOURCE_SERVER_ID=scipro-api-client
- RESOURCE_SERVER_SECRET=scipro-api-secret
+ oauth2-wicket:
+ build:
+ context: https://github.com/dsv-su/toker.git
+ dockerfile: embedded.Dockerfile
+ restart: on-failure
+ ports:
+ - '59734:8080'
+ environment:
+ - CLIENT_ID=scipro
+ - CLIENT_SECRET=s3cr3t
+ - CLIENT_REDIRECT_URI=http://localhost:8080/login/oauth2/code/scipro
diff --git a/view/src/main/java/se/su/dsv/scipro/CurrentUserFromWicketSession.java b/view/src/main/java/se/su/dsv/scipro/CurrentUserFromWicketSession.java
deleted file mode 100644
index a13b465884..0000000000
--- a/view/src/main/java/se/su/dsv/scipro/CurrentUserFromWicketSession.java
+++ /dev/null
@@ -1,14 +0,0 @@
-package se.su.dsv.scipro;
-
-import org.apache.wicket.Session;
-import se.su.dsv.scipro.session.SciProSession;
-import se.su.dsv.scipro.system.CurrentUser;
-import se.su.dsv.scipro.system.User;
-
-public class CurrentUserFromWicketSession implements CurrentUser {
-
- @Override
- public User get() {
- return Session.exists() ? SciProSession.get().getUser() : null;
- }
-}
diff --git a/view/src/main/java/se/su/dsv/scipro/loginlogout/pages/SSOPage.java b/view/src/main/java/se/su/dsv/scipro/loginlogout/pages/SSOPage.java
index 8df12a7cce..cb9a802c52 100644
--- a/view/src/main/java/se/su/dsv/scipro/loginlogout/pages/SSOPage.java
+++ b/view/src/main/java/se/su/dsv/scipro/loginlogout/pages/SSOPage.java
@@ -1,12 +1,12 @@
package se.su.dsv.scipro.loginlogout.pages;
import jakarta.inject.Inject;
-import jakarta.servlet.http.HttpServletRequest;
import java.util.Optional;
import java.util.Set;
import se.su.dsv.scipro.basepages.PublicPage;
import se.su.dsv.scipro.security.auth.Authorization;
import se.su.dsv.scipro.session.SciProSession;
+import se.su.dsv.scipro.system.AuthenticationContext;
import se.su.dsv.scipro.system.User;
import se.su.dsv.scipro.system.UserImportService;
import se.su.dsv.scipro.system.UserService;
@@ -20,8 +20,11 @@ public class SSOPage extends PublicPage {
@Inject
private UserService userService;
+ @Inject
+ private AuthenticationContext authenticationContext;
+
public SSOPage() {
- String remoteUserName = ((HttpServletRequest) getRequest().getContainerRequest()).getRemoteUser();
+ String remoteUserName = authenticationContext.getPrincipalName();
User user = userService.findByUsername(remoteUserName);
if (user != null) {
diff --git a/view/src/main/java/se/su/dsv/scipro/security/auth/MockRemoteUserFilter.java b/view/src/main/java/se/su/dsv/scipro/security/auth/MockRemoteUserFilter.java
deleted file mode 100755
index c8ab375393..0000000000
--- a/view/src/main/java/se/su/dsv/scipro/security/auth/MockRemoteUserFilter.java
+++ /dev/null
@@ -1,81 +0,0 @@
-package se.su.dsv.scipro.security.auth;
-
-import jakarta.servlet.*;
-import jakarta.servlet.http.HttpServletRequest;
-import jakarta.servlet.http.HttpServletRequestWrapper;
-import java.io.IOException;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-/**
- * Throw-away implementation of a servlet filter, main task is to fake the getRemoteUser() call for the request chain.
- */
-public final class MockRemoteUserFilter implements Filter {
-
- private static final Logger LOGGER = LoggerFactory.getLogger(MockRemoteUserFilter.class);
- //Default value unless supplied via init parameter
- private String fakedUser = "SOME_GUY";
- private FilterConfig cfg = null;
-
- /**
- * Default constructor.
- */
- public MockRemoteUserFilter() {}
-
- /**
- * @see Filter#destroy()
- */
- @Override
- public void destroy() {
- cfg = null;
- }
-
- /**
- * Wraps the passed request and alters the behavior of getRemoteUser() for later links of the chain.
- * @see Filter#doFilter(ServletRequest, ServletResponse, FilterChain)
- */
- @Override
- public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
- throws IOException, ServletException {
- LOGGER.debug("Faking external authentication user: " + fakedUser);
- if (cfg != null) {
- HttpServletRequestWrapper wrapper = new ModifiedRemoteUserRequestWrapper(
- (HttpServletRequest) request,
- fakedUser
- );
- // pass the request along the filter chain
- chain.doFilter(wrapper, response);
- return;
- }
- chain.doFilter(request, response);
- }
-
- /**
- * @see Filter#init(FilterConfig)
- */
- @Override
- public void init(FilterConfig fConfig) {
- cfg = fConfig;
- if (cfg != null) {
- fakedUser = cfg.getInitParameter("fakedUser");
- }
- }
-
- /**
- * Private RequestWrapper, of no interest to anyone outside of this class.
- */
- static class ModifiedRemoteUserRequestWrapper extends HttpServletRequestWrapper {
-
- private final String fakedUser;
-
- ModifiedRemoteUserRequestWrapper(final HttpServletRequest request, final String fakedUser) {
- super(request);
- this.fakedUser = fakedUser;
- }
-
- @Override
- public String getRemoteUser() {
- return fakedUser;
- }
- }
-}
diff --git a/view/src/main/java/se/su/dsv/scipro/session/SciProSession.java b/view/src/main/java/se/su/dsv/scipro/session/SciProSession.java
index 75af2b3bff..202e061242 100755
--- a/view/src/main/java/se/su/dsv/scipro/session/SciProSession.java
+++ b/view/src/main/java/se/su/dsv/scipro/session/SciProSession.java
@@ -5,24 +5,21 @@ import java.util.Collections;
import java.util.HashSet;
import java.util.Locale;
import java.util.Set;
-import org.apache.wicket.MetaDataKey;
import org.apache.wicket.Session;
import org.apache.wicket.injection.Injector;
import org.apache.wicket.protocol.http.WebSession;
import org.apache.wicket.request.Request;
import se.su.dsv.scipro.security.auth.roles.IRole;
import se.su.dsv.scipro.security.auth.roles.Roles;
+import se.su.dsv.scipro.system.AuthenticationContext;
import se.su.dsv.scipro.system.ProjectModule;
import se.su.dsv.scipro.system.SystemModule;
import se.su.dsv.scipro.system.User;
-import se.su.dsv.scipro.system.UserService;
public class SciProSession extends WebSession {
- private static final MetaDataKey<Long> LOGGED_IN_USER_ID = new MetaDataKey<>() {};
-
@Inject
- private UserService userService;
+ private AuthenticationContext authenticationContext;
private Set<ProjectModule> projectModules = new HashSet<>();
private Set<SystemModule> systemModules = new HashSet<>();
@@ -37,15 +34,15 @@ public class SciProSession extends WebSession {
}
public synchronized void setUser(User user) {
- setMetaData(LOGGED_IN_USER_ID, user.getId());
+ authenticationContext.set(user);
}
public synchronized User getUser() {
- return isLoggedIn() ? userService.findOne(getMetaData(LOGGED_IN_USER_ID)) : null;
+ return authenticationContext.get();
}
public synchronized boolean isLoggedIn() {
- return getMetaData(LOGGED_IN_USER_ID) != null;
+ return authenticationContext.get() != null;
}
public synchronized boolean authorizedForRole(Roles role) {
diff --git a/view/src/test/java/se/su/dsv/scipro/SciProTest.java b/view/src/test/java/se/su/dsv/scipro/SciProTest.java
index ceec7ebadb..768b84e0df 100755
--- a/view/src/test/java/se/su/dsv/scipro/SciProTest.java
+++ b/view/src/test/java/se/su/dsv/scipro/SciProTest.java
@@ -3,7 +3,6 @@ package se.su.dsv.scipro;
import static org.mockito.ArgumentMatchers.*;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
import com.google.common.eventbus.EventBus;
import java.lang.reflect.Field;
@@ -119,12 +118,11 @@ import se.su.dsv.scipro.springdata.services.UnitService;
import se.su.dsv.scipro.springdata.services.UserProfileService;
import se.su.dsv.scipro.supervisor.pages.SupervisorStartPage;
import se.su.dsv.scipro.survey.SurveyService;
-import se.su.dsv.scipro.system.CurrentUser;
+import se.su.dsv.scipro.system.AuthenticationContext;
import se.su.dsv.scipro.system.ExternalResourceService;
import se.su.dsv.scipro.system.FooterAddressRepo;
import se.su.dsv.scipro.system.FooterLinkService;
import se.su.dsv.scipro.system.GenericService;
-import se.su.dsv.scipro.system.Lifecycle;
import se.su.dsv.scipro.system.PasswordRepo;
import se.su.dsv.scipro.system.PasswordService;
import se.su.dsv.scipro.system.ProjectModule;
@@ -369,7 +367,7 @@ public abstract class SciProTest {
protected ChecklistAnswerService checklistAnswerService;
@Mock
- protected CurrentUser currentUser;
+ protected AuthenticationContext authenticationContext;
@Mock
private Scheduler scheduler;
diff --git a/war/pom.xml b/war/pom.xml
index 2c4ad17242..456eeae393 100644
--- a/war/pom.xml
+++ b/war/pom.xml
@@ -41,6 +41,10 @@
<artifactId>spring-boot-starter-tomcat</artifactId>
<scope>provided</scope>
</dependency>
+ <dependency>
+ <groupId>org.springframework.boot</groupId>
+ <artifactId>spring-boot-starter-oauth2-client</artifactId>
+ </dependency>
<dependency>
<groupId>org.springframework</groupId>
<artifactId>spring-orm</artifactId>
diff --git a/war/src/main/java/se/su/dsv/scipro/war/CurrentUserFromSpringSecurity.java b/war/src/main/java/se/su/dsv/scipro/war/CurrentUserFromSpringSecurity.java
new file mode 100644
index 0000000000..6f209f38aa
--- /dev/null
+++ b/war/src/main/java/se/su/dsv/scipro/war/CurrentUserFromSpringSecurity.java
@@ -0,0 +1,98 @@
+package se.su.dsv.scipro.war;
+
+import jakarta.inject.Inject;
+import jakarta.inject.Provider;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import java.security.Principal;
+import java.util.Collections;
+import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
+import org.springframework.security.core.Authentication;
+import org.springframework.security.core.context.SecurityContext;
+import org.springframework.security.core.context.SecurityContextHolder;
+import org.springframework.security.core.context.SecurityContextHolderStrategy;
+import org.springframework.security.web.context.HttpSessionSecurityContextRepository;
+import org.springframework.security.web.context.SecurityContextRepository;
+import se.su.dsv.scipro.system.AuthenticationContext;
+import se.su.dsv.scipro.system.User;
+import se.su.dsv.scipro.system.UserService;
+import se.su.dsv.scipro.system.Username;
+
+public class CurrentUserFromSpringSecurity implements AuthenticationContext {
+
+ private final UserService userService;
+
+ // injecting providers since this is a singleton and the request and response are not
+ private final Provider<HttpServletRequest> currentRequest;
+ private final Provider<HttpServletResponse> currentResponse;
+
+ // hardcoded since that is what Spring Security does (see SwitchUserFilter)
+ private final SecurityContextRepository securityContextRepository = new HttpSessionSecurityContextRepository();
+
+ @Inject
+ public CurrentUserFromSpringSecurity(
+ UserService userService,
+ Provider<HttpServletRequest> currentRequest,
+ Provider<HttpServletResponse> currentResponse
+ ) {
+ this.userService = userService;
+ this.currentRequest = currentRequest;
+ this.currentResponse = currentResponse;
+ }
+
+ @Override
+ public User get() {
+ SecurityContext context = SecurityContextHolder.getContext();
+ Authentication authentication = context.getAuthentication();
+ if (authentication == null) {
+ return null;
+ }
+ String username = authentication.getName();
+ return userService.findByUsername(username);
+ }
+
+ // Implementing switch user manually rather than using the built-in Spring Security switch user feature
+ // due to compatibility with Wicket.
+ // Wicket does not supply a form with a username field since it has some JavaScript based auto-complete
+ // person finder.
+ // See Spring's SwitchUserFilter for the built-in switch user feature from where most of the code is copied.
+ @Override
+ public void set(User user) {
+ SecurityContextHolderStrategy strategy = SecurityContextHolder.getContextHolderStrategy();
+ SecurityContext context = strategy.createEmptyContext();
+ WicketControlledPrincipal principal = new WicketControlledPrincipal(user);
+ UsernamePasswordAuthenticationToken targetUser = UsernamePasswordAuthenticationToken.authenticated(
+ principal,
+ null,
+ Collections.emptyList()
+ );
+ context.setAuthentication(targetUser);
+ strategy.setContext(context);
+ this.securityContextRepository.saveContext(context, currentRequest.get(), currentResponse.get());
+ }
+
+ @Override
+ public String getPrincipalName() {
+ SecurityContext context = SecurityContextHolder.getContext();
+ Authentication authentication = context.getAuthentication();
+ if (authentication == null) {
+ return null;
+ }
+ return authentication.getName();
+ }
+
+ private static final class WicketControlledPrincipal implements Principal {
+
+ private final String username;
+
+ public WicketControlledPrincipal(User user) {
+ // extract any username so that we can look it up later
+ this.username = user.getUsernames().stream().findAny().map(Username::getUsername).orElse("<unknown>");
+ }
+
+ @Override
+ public String getName() {
+ return username;
+ }
+ }
+}
diff --git a/war/src/main/java/se/su/dsv/scipro/war/Main.java b/war/src/main/java/se/su/dsv/scipro/war/Main.java
index 9669f121d6..2a14e64915 100644
--- a/war/src/main/java/se/su/dsv/scipro/war/Main.java
+++ b/war/src/main/java/se/su/dsv/scipro/war/Main.java
@@ -23,7 +23,6 @@ import org.springframework.core.task.SimpleAsyncTaskExecutor;
import org.springframework.orm.jpa.SharedEntityManagerCreator;
import org.springframework.orm.jpa.support.OpenEntityManagerInViewFilter;
import se.su.dsv.scipro.CoreConfig;
-import se.su.dsv.scipro.CurrentUserFromWicketSession;
import se.su.dsv.scipro.FileSystemStore;
import se.su.dsv.scipro.RepositoryConfiguration;
import se.su.dsv.scipro.file.FileStore;
@@ -85,11 +84,6 @@ public class Main extends SpringBootServletInitializer implements ServletContain
return currentProfile;
}
- @Bean
- public CurrentUserFromWicketSession currentUserFromWicketSession() {
- return new CurrentUserFromWicketSession();
- }
-
@Bean
public FileStore fileStore() {
return new FileSystemStore();
diff --git a/war/src/main/java/se/su/dsv/scipro/war/WicketConfiguration.java b/war/src/main/java/se/su/dsv/scipro/war/WicketConfiguration.java
index db84241566..90b0b70a61 100644
--- a/war/src/main/java/se/su/dsv/scipro/war/WicketConfiguration.java
+++ b/war/src/main/java/se/su/dsv/scipro/war/WicketConfiguration.java
@@ -1,6 +1,9 @@
package se.su.dsv.scipro.war;
import com.google.common.eventbus.EventBus;
+import jakarta.inject.Provider;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
import org.apache.wicket.protocol.http.WebApplication;
import org.apache.wicket.protocol.http.WicketFilter;
import org.apache.wicket.spring.injection.annot.SpringComponentInjector;
@@ -8,6 +11,10 @@ import org.springframework.boot.web.servlet.FilterRegistrationBean;
import org.springframework.context.ApplicationContext;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
+import org.springframework.core.annotation.Order;
+import org.springframework.security.config.Customizer;
+import org.springframework.security.config.annotation.web.builders.HttpSecurity;
+import org.springframework.security.web.SecurityFilterChain;
import se.su.dsv.scipro.SciProApplication;
import se.su.dsv.scipro.crosscutting.ForwardPhase2Feedback;
import se.su.dsv.scipro.crosscutting.NotifyFailedReflection;
@@ -21,6 +28,7 @@ import se.su.dsv.scipro.notifications.NotificationController;
import se.su.dsv.scipro.profiles.CurrentProfile;
import se.su.dsv.scipro.reviewing.FinalSeminarApprovalService;
import se.su.dsv.scipro.reviewing.RoughDraftApprovalService;
+import se.su.dsv.scipro.system.UserService;
@Configuration
public class WicketConfiguration {
@@ -49,6 +57,25 @@ public class WicketConfiguration {
return new SciProApplication(currentProfile);
}
+ @Bean
+ @Order(3) // make sure it's after the API security filters
+ public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
+ http.authorizeHttpRequests(requests -> requests.anyRequest().authenticated());
+ http.oauth2Login(Customizer.withDefaults());
+ http.csrf(csrf -> csrf.disable()); // Wicket has its own CSRF protection
+ http.logout(logout -> logout.logoutUrl("/logout").logoutSuccessUrl("/"));
+ return http.build();
+ }
+
+ @Bean
+ public CurrentUserFromSpringSecurity currentUserFromSpringSecurity(
+ UserService userService,
+ Provider<HttpServletRequest> httpServletRequestProvider,
+ Provider<HttpServletResponse> httpServletResponseProvider
+ ) {
+ return new CurrentUserFromSpringSecurity(userService, httpServletRequestProvider, httpServletResponseProvider);
+ }
+
@Bean
public ReviewingNotifications reviewingNotifications(
EventBus eventBus,
diff --git a/war/src/main/resources/application.properties b/war/src/main/resources/application.properties
index 7754344e7d..f405136272 100644
--- a/war/src/main/resources/application.properties
+++ b/war/src/main/resources/application.properties
@@ -19,3 +19,14 @@ springdoc.swagger-ui.persist-authorization=true
spring.security.oauth2.resourceserver.opaquetoken.client-id=scipro-api-client
spring.security.oauth2.resourceserver.opaquetoken.client-secret=scipro-api-secret
spring.security.oauth2.resourceserver.opaquetoken.introspection-uri=http://localhost:59733/introspect
+
+# Log in via local OAuth 2 authorization server
+spring.security.oauth2.client.provider.docker.user-info-uri=http://localhost:59734/verify
+spring.security.oauth2.client.provider.docker.user-name-attribute=sub
+spring.security.oauth2.client.provider.docker.token-uri=http://localhost:59734/exchange
+spring.security.oauth2.client.provider.docker.authorization-uri=http://localhost:59734/authorize
+spring.security.oauth2.client.registration.scipro.redirect-uri={baseUrl}/login/oauth2/code/{registrationId}
+spring.security.oauth2.client.registration.scipro.provider=docker
+spring.security.oauth2.client.registration.scipro.client-id=scipro
+spring.security.oauth2.client.registration.scipro.client-secret=s3cr3t
+spring.security.oauth2.client.registration.scipro.authorization-grant-type=authorization_code