From de18f200a7b95ad9e1e7bc9026335f80bd2a4412 Mon Sep 17 00:00:00 2001
From: Andreas Svanberg <andreass@dsv.su.se>
Date: Mon, 10 Mar 2025 07:17:28 +0100
Subject: [PATCH 1/9] Make exemptions for project type apply to partner as well
(#128)
Read #119 for context first
When authors select supervisor ideas during an open application period they are only allowed to select ideas corresponding to their degree type classification. This limitation can be lifted by giving the author an exemption for "Project type limitation" on the corresponding application period. However, this limitation is still enforced for any potential partner *even if* the have been given the same exemption. This change makes it so the exemption applies to any selected partner as well and not just the author selecting the supervisor idea.
## How to test
1. Log in as `oskar@example.com`
2. Go to "Ideas / My ideas" page
3. Click "Select from available ideas" on the application period "Supervisor ideas"
4. Open the one available idea
5. Try to select it with "Johan Student" as a partner
6. Log in as admin
7. Go to "Match / Application periods"
8. Click "Edit exemptions" on the "Supervisor ideas" period
9. Give "Johan Student" an exemption to "Project type limitation"
10. Repeat steps 1-5
Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/128
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/match/IdeaServiceImpl.java | 6 +-
.../dsv/scipro/match/IdeaServiceImplTest.java | 34 ++++++++++
.../se/su/dsv/scipro/testdata/BaseData.java | 10 +++
.../dsv/scipro/testdata/DataInitializer.java | 5 ++
.../populators/PartnerTypeExemption.java | 68 +++++++++++++++++++
5 files changed, 119 insertions(+), 4 deletions(-)
create mode 100644 test-data/src/main/java/se/su/dsv/scipro/testdata/populators/PartnerTypeExemption.java
diff --git a/core/src/main/java/se/su/dsv/scipro/match/IdeaServiceImpl.java b/core/src/main/java/se/su/dsv/scipro/match/IdeaServiceImpl.java
index 08845a8a3f..2e97a2b07a 100755
--- a/core/src/main/java/se/su/dsv/scipro/match/IdeaServiceImpl.java
+++ b/core/src/main/java/se/su/dsv/scipro/match/IdeaServiceImpl.java
@@ -235,10 +235,8 @@ public class IdeaServiceImpl extends AbstractServiceImpl<Idea, Long> implements
if (authorParticipatingOnActiveIdea(coAuthor, ap)) {
return new Pair<>(Boolean.FALSE, PARTNER_ALREADY_PARTICIPATING_ERROR);
}
- if (
- coAuthor.getDegreeType() != ProjectType.UNKNOWN &&
- coAuthor.getDegreeType() != idea.getProjectType().getDegreeType()
- ) {
+ List<ProjectType> typesForCoAuthor = applicationPeriodService.getTypesForStudent(ap, coAuthor);
+ if (!typesForCoAuthor.contains(idea.getProjectType())) {
return new Pair<>(Boolean.FALSE, WRONG_LEVEL_FOR_YOUR_PARTNER);
}
if (!projectService.getActiveProjectsByUserAndProjectType(coAuthor, idea.getProjectType()).isEmpty()) {
diff --git a/core/src/test/java/se/su/dsv/scipro/match/IdeaServiceImplTest.java b/core/src/test/java/se/su/dsv/scipro/match/IdeaServiceImplTest.java
index 6365ba546a..871ea15351 100755
--- a/core/src/test/java/se/su/dsv/scipro/match/IdeaServiceImplTest.java
+++ b/core/src/test/java/se/su/dsv/scipro/match/IdeaServiceImplTest.java
@@ -241,6 +241,7 @@ public class IdeaServiceImplTest {
when(generalSystemSettingsService.getGeneralSystemSettingsInstance()).thenReturn(new GeneralSystemSettings());
Idea idea = createBachelorIdea(Idea.Status.UNMATCHED);
when(applicationPeriodService.getTypesForStudent(applicationPeriod, student)).thenReturn(List.of(bachelor));
+ when(applicationPeriodService.getTypesForStudent(applicationPeriod, coAuthor)).thenReturn(List.of(bachelor));
Pair<Boolean, String> acceptance = ideaService.validateStudentAcceptance(
idea,
@@ -401,6 +402,39 @@ public class IdeaServiceImplTest {
assertEquals(expected, ideaService.countAuthorsByApplicationPeriod(applicationPeriod, params));
}
+ @Test
+ public void wrong_type_for_author() {
+ when(applicationPeriodService.getTypesForStudent(applicationPeriod, student)).thenReturn(List.of(master));
+ when(applicationPeriodService.getTypesForStudent(applicationPeriod, coAuthor)).thenReturn(List.of(bachelor));
+
+ assertPair(
+ false,
+ "The idea is the wrong level for you, please pick another one.",
+ ideaService.validateStudentAcceptance(
+ createBachelorIdea(Idea.Status.UNMATCHED),
+ student,
+ coAuthor,
+ applicationPeriod
+ )
+ );
+ }
+
+ @Test
+ public void wrong_type_for_partner() {
+ when(applicationPeriodService.getTypesForStudent(applicationPeriod, coAuthor)).thenReturn(List.of(master));
+
+ assertPair(
+ false,
+ "The idea is the wrong level for your partner, please pick another one.",
+ ideaService.validateStudentAcceptance(
+ createBachelorIdea(Idea.Status.UNMATCHED),
+ student,
+ coAuthor,
+ applicationPeriod
+ )
+ );
+ }
+
private Idea mockInactiveIdea() {
Idea idea = new Idea();
Match match = new Match();
diff --git a/test-data/src/main/java/se/su/dsv/scipro/testdata/BaseData.java b/test-data/src/main/java/se/su/dsv/scipro/testdata/BaseData.java
index 1dd6b18132..56bb1313a4 100644
--- a/test-data/src/main/java/se/su/dsv/scipro/testdata/BaseData.java
+++ b/test-data/src/main/java/se/su/dsv/scipro/testdata/BaseData.java
@@ -1,6 +1,9 @@
package se.su.dsv.scipro.testdata;
+import java.util.List;
+import se.su.dsv.scipro.match.Keyword;
import se.su.dsv.scipro.system.ProjectType;
+import se.su.dsv.scipro.system.ResearchArea;
/// All the base test data that can be re-used in different test cases.
///
@@ -16,4 +19,11 @@ public interface BaseData {
ProjectType bachelor();
ProjectType magister();
ProjectType master();
+
+ /**
+ * @return generic research area with some keywords attached to it
+ */
+ ResearchAreaAndKeywords researchArea();
+
+ record ResearchAreaAndKeywords(ResearchArea researchArea, List<Keyword> keywords) {}
}
diff --git a/test-data/src/main/java/se/su/dsv/scipro/testdata/DataInitializer.java b/test-data/src/main/java/se/su/dsv/scipro/testdata/DataInitializer.java
index cf01ab2315..c12cb602b2 100644
--- a/test-data/src/main/java/se/su/dsv/scipro/testdata/DataInitializer.java
+++ b/test-data/src/main/java/se/su/dsv/scipro/testdata/DataInitializer.java
@@ -2193,6 +2193,11 @@ public class DataInitializer implements Lifecycle, BaseData, Factory {
return createEmployee(firstName);
}
+ @Override
+ public ResearchAreaAndKeywords researchArea() {
+ return new ResearchAreaAndKeywords(researchArea1, List.of(keyword1, keyword2));
+ }
+
private static final class SimpleTextFile implements FileUpload {
private final User uploader;
diff --git a/test-data/src/main/java/se/su/dsv/scipro/testdata/populators/PartnerTypeExemption.java b/test-data/src/main/java/se/su/dsv/scipro/testdata/populators/PartnerTypeExemption.java
new file mode 100644
index 0000000000..c490d0b069
--- /dev/null
+++ b/test-data/src/main/java/se/su/dsv/scipro/testdata/populators/PartnerTypeExemption.java
@@ -0,0 +1,68 @@
+package se.su.dsv.scipro.testdata.populators;
+
+import jakarta.inject.Inject;
+import java.time.LocalDate;
+import java.time.LocalDateTime;
+import java.util.Set;
+import org.springframework.stereotype.Service;
+import se.su.dsv.scipro.match.ApplicationPeriod;
+import se.su.dsv.scipro.match.ApplicationPeriodService;
+import se.su.dsv.scipro.match.Idea;
+import se.su.dsv.scipro.match.IdeaService;
+import se.su.dsv.scipro.match.Target;
+import se.su.dsv.scipro.match.TargetService;
+import se.su.dsv.scipro.system.User;
+import se.su.dsv.scipro.testdata.BaseData;
+import se.su.dsv.scipro.testdata.Factory;
+import se.su.dsv.scipro.testdata.TestDataPopulator;
+
+@Service
+public class PartnerTypeExemption implements TestDataPopulator {
+
+ private final ApplicationPeriodService applicationPeriodService;
+ private final IdeaService ideaService;
+ private final TargetService targetService;
+
+ @Inject
+ public PartnerTypeExemption(
+ ApplicationPeriodService applicationPeriodService,
+ IdeaService ideaService,
+ TargetService targetService
+ ) {
+ this.applicationPeriodService = applicationPeriodService;
+ this.ideaService = ideaService;
+ this.targetService = targetService;
+ }
+
+ @Override
+ public void populate(BaseData baseData, Factory factory) {
+ factory.createAuthor("Oskar");
+
+ User johan = factory.createAuthor("Johan");
+ johan.setDegreeType(baseData.master().getDegreeType());
+
+ User supervisor = factory.createSupervisor("Elsa");
+
+ ApplicationPeriod applicationPeriod = new ApplicationPeriod("Supervisor ideas");
+ applicationPeriod.setStartDate(LocalDate.now());
+ applicationPeriod.setEndDate(LocalDate.now().plusDays(14));
+ applicationPeriod.setCourseStartDateTime(LocalDateTime.now().plusDays(15));
+ applicationPeriod.setProjectTypes(Set.of(baseData.bachelor()));
+ applicationPeriodService.save(applicationPeriod);
+
+ Target target = targetService.findOne(applicationPeriod, supervisor, baseData.bachelor());
+ target.setTarget(10);
+ targetService.save(target);
+
+ Idea idea = new Idea();
+ idea.setPublished(true);
+ idea.setTitle("The next gen AI 2.0 turbo edition");
+ idea.setPrerequisites("Hacker experience");
+ idea.setDescription("Better than all the rest");
+ idea.setProjectType(baseData.bachelor());
+ idea.setApplicationPeriod(applicationPeriod);
+ idea.setResearchArea(baseData.researchArea().researchArea());
+
+ ideaService.saveSupervisorIdea(idea, supervisor, baseData.researchArea().keywords(), true);
+ }
+}
From 9ede262e7bd7767933e0ce4a349bb73436a8786b Mon Sep 17 00:00:00 2001
From: Andreas Svanberg <andreass@dsv.su.se>
Date: Tue, 11 Mar 2025 08:49:16 +0100
Subject: [PATCH 2/9] Fix crash due to JSON parsing on "Finishing up" tab
(#132)
The seemingly unused library `jersey-hk2` that got removed in #118 is used, if present, internally by the Jersey client to find and register Jackson modules (such as those that provide `java.time` support).
## How to test
1. Turn on the `DAISY-INTEGRATION` Maven profile (alongside `DEV`)
2. Configure some projects and their authors to have a Daisy connection
3. Log in as the supervisor
4. Go to the "Finishing up" tab in the project
5. See that it works compared to `develop` branch
Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/132
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>
---
core/pom.xml | 4 ++
...adingReportServiceImplIntegrationTest.java | 45 +++++++++++++++++++
2 files changed, 49 insertions(+)
diff --git a/core/pom.xml b/core/pom.xml
index d2a9506ee7..cda6ec6cdd 100644
--- a/core/pom.xml
+++ b/core/pom.xml
@@ -32,6 +32,10 @@
<groupId>org.glassfish.jersey.media</groupId>
<artifactId>jersey-media-json-jackson</artifactId>
</dependency>
+ <dependency>
+ <groupId>org.glassfish.jersey.inject</groupId>
+ <artifactId>jersey-hk2</artifactId>
+ </dependency>
<dependency>
<groupId>org.springframework</groupId>
<artifactId>spring-context</artifactId>
diff --git a/core/src/test/java/se/su/dsv/scipro/report/GradingReportServiceImplIntegrationTest.java b/core/src/test/java/se/su/dsv/scipro/report/GradingReportServiceImplIntegrationTest.java
index 597396bbea..63808671e3 100644
--- a/core/src/test/java/se/su/dsv/scipro/report/GradingReportServiceImplIntegrationTest.java
+++ b/core/src/test/java/se/su/dsv/scipro/report/GradingReportServiceImplIntegrationTest.java
@@ -6,7 +6,11 @@ import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import com.google.common.eventbus.EventBus;
+import com.sun.net.httpserver.HttpServer;
import jakarta.inject.Inject;
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.nio.charset.StandardCharsets;
import java.time.LocalDate;
import java.time.Month;
import java.util.*;
@@ -15,6 +19,9 @@ import org.junit.jupiter.api.Test;
import se.su.dsv.scipro.finalseminar.FinalSeminar;
import se.su.dsv.scipro.finalseminar.FinalSeminarOpposition;
import se.su.dsv.scipro.finalseminar.OppositionApprovedEvent;
+import se.su.dsv.scipro.grading.GetGradeError;
+import se.su.dsv.scipro.grading.GradingServiceImpl;
+import se.su.dsv.scipro.grading.Result;
import se.su.dsv.scipro.project.Project;
import se.su.dsv.scipro.security.auth.roles.Roles;
import se.su.dsv.scipro.system.DegreeType;
@@ -149,6 +156,44 @@ public class GradingReportServiceImplIntegrationTest extends IntegrationTest {
assertNull(oppositionCriterion.getFeedback());
}
+ @Test
+ public void test_json_deserialization() throws IOException {
+ String json =
+ """
+ {
+ "grade": "A",
+ "reported": "2021-01-01"
+ }
+ """;
+ HttpServer httpServer = startHttpServerWithJsonResponse(json);
+
+ int port = httpServer.getAddress().getPort();
+
+ GradingServiceImpl gradingService = new GradingServiceImpl("http://localhost:" + port);
+ Either<GetGradeError, Optional<Result>> result = gradingService.getResult("token", 1, 2, 3);
+
+ Optional<Result> right = result.right();
+ assertTrue(right.isPresent());
+ assertEquals(LocalDate.of(2021, 1, 1), right.get().reported());
+
+ httpServer.stop(0);
+ }
+
+ private static HttpServer startHttpServerWithJsonResponse(String json) throws IOException {
+ HttpServer httpServer = HttpServer.create();
+ httpServer.createContext("/", exchange -> {
+ try (exchange) {
+ byte[] response = json.getBytes(StandardCharsets.UTF_8);
+ exchange.getResponseHeaders().add("Content-Type", "application/json");
+ exchange.sendResponseHeaders(200, response.length);
+ exchange.getResponseBody().write(response);
+ }
+ });
+ httpServer.bind(new InetSocketAddress("localhost", 0), 0);
+ httpServer.start();
+ return httpServer;
+ }
+
private void addOppositionCriterion() {
gradingReportTemplate = createOppositionCriteria(gradingReportTemplate, 2);
gradingReport = createGradingReport(project, student);
From 59e3ec3fd940bf257546d67301233979c27de239 Mon Sep 17 00:00:00 2001
From: Andreas Svanberg <andreass@dsv.su.se>
Date: Tue, 11 Mar 2025 09:15:01 +0100
Subject: [PATCH 3/9] Maintain project selection on validation failure during
group creation (#133)
Fixes #129
## How to test
1. Log in as `evan@example.com`
2. Go to "My groups"
3. Click "Create new group"
4. Select some projects but do *not* fill in the "Title"
5. Click save
6. Error message should be presented
7. Project selection should be maintained
Co-authored-by: Nico Athanassiadis <nico@dsv.su.se>
Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/133
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>
---
.../src/main/java/se/su/dsv/scipro/group/EditGroupPanel.java | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/view/src/main/java/se/su/dsv/scipro/group/EditGroupPanel.java b/view/src/main/java/se/su/dsv/scipro/group/EditGroupPanel.java
index 831986ffad..05b8396ffa 100644
--- a/view/src/main/java/se/su/dsv/scipro/group/EditGroupPanel.java
+++ b/view/src/main/java/se/su/dsv/scipro/group/EditGroupPanel.java
@@ -59,6 +59,11 @@ public class EditGroupPanel extends Panel {
});
add(
new ListView<>("available_projects", availableProjects) {
+ {
+ // must re-use list items to maintain form component (checkboxes) state
+ setReuseItems(true);
+ }
+
@Override
protected void populateItem(ListItem<Project> item) {
CheckBox checkbox = new CheckBox("selected", new SelectProjectModel(model, item.getModel()));
From 6b77142a067597557522007a51bd928fdc394153 Mon Sep 17 00:00:00 2001
From: Andreas Svanberg <andreass@dsv.su.se>
Date: Tue, 18 Mar 2025 07:33:38 +0100
Subject: [PATCH 4/9] New Daisy API XSD (#134)
Allows a way to solve #119
Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/134
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>
---
core/src/main/xsd/daisy_api.xsd | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/core/src/main/xsd/daisy_api.xsd b/core/src/main/xsd/daisy_api.xsd
index fa68eef856..7ff97ec5a7 100755
--- a/core/src/main/xsd/daisy_api.xsd
+++ b/core/src/main/xsd/daisy_api.xsd
@@ -559,7 +559,7 @@
<xs:complexType name="course">
<xs:sequence>
- <xs:element name="courseCode" type="xs:string" minOccurs="0">
+ <xs:element name="courseCode" type="xs:string" minOccurs="1">
</xs:element>
<xs:element name="credits" type="xs:float" minOccurs="1">
</xs:element>
@@ -567,6 +567,8 @@
</xs:element>
<xs:element name="level" type="educationalLevel" minOccurs="0">
</xs:element>
+ <xs:element name="degreeThesisCourse" type="xs:boolean" minOccurs="1">
+ </xs:element>
<xs:element name="eduInstDesignation" type="xs:string" minOccurs="1">
</xs:element>
</xs:sequence>
From 7504c267c5a3f6faefd590f770347d6ce38b97fe Mon Sep 17 00:00:00 2001
From: Andreas Svanberg <andreass@dsv.su.se>
Date: Tue, 18 Mar 2025 08:34:21 +0100
Subject: [PATCH 5/9] Delete forum replies (#137)
Allows deleting (your own) forum replies.
Fixes #90
## How to test
1. Log in as `eric@example.com` (supervisor) or as `sture@example.com` (author)
2. Open the forum in project "Putting the it in supervising"
3. Create a new thread
4. Post some replies as the different users
5. Delete the replies
Co-authored-by: Nico Athanassiadis <nico@dsv.su.se>
Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/137
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>
---
.../java/se/su/dsv/scipro/CoreConfig.java | 6 +-
.../dsv/scipro/forum/BasicForumService.java | 8 ++
.../scipro/forum/BasicForumServiceImpl.java | 60 +++++++++++++-
.../scipro/forum/dataobjects/ForumThread.java | 9 --
.../forum/BasicForumServiceImplTest.java | 2 +
.../BasicForumServiceIntegrationTest.java | 83 +++++++++++++++++++
.../se/su/dsv/scipro/test/SpringTest.java | 27 +++++-
.../forum/panels/threaded/ForumPostPanel.html | 5 +-
.../forum/panels/threaded/ForumPostPanel.java | 30 +++++++
.../panels/threaded/ThreadsOverviewPanel.java | 16 ++--
.../panels/threaded/ViewForumThreadPanel.java | 16 +++-
.../threaded/ThreadsOverviewPanelTest.java | 5 +-
12 files changed, 241 insertions(+), 26 deletions(-)
create mode 100644 core/src/test/java/se/su/dsv/scipro/forum/BasicForumServiceIntegrationTest.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 352c43456a..bdcf367030 100644
--- a/core/src/main/java/se/su/dsv/scipro/CoreConfig.java
+++ b/core/src/main/java/se/su/dsv/scipro/CoreConfig.java
@@ -349,14 +349,16 @@ public class CoreConfig {
ForumPostReadStateRepository readStateRepository,
AbstractThreadRepository threadRepository,
FileService fileService,
- EventBus eventBus
+ EventBus eventBus,
+ CurrentUser currentUser
) {
return new BasicForumServiceImpl(
forumPostRepository,
readStateRepository,
threadRepository,
fileService,
- eventBus
+ eventBus,
+ currentUser
);
}
diff --git a/core/src/main/java/se/su/dsv/scipro/forum/BasicForumService.java b/core/src/main/java/se/su/dsv/scipro/forum/BasicForumService.java
index 6a680ad26f..44ff9f407a 100644
--- a/core/src/main/java/se/su/dsv/scipro/forum/BasicForumService.java
+++ b/core/src/main/java/se/su/dsv/scipro/forum/BasicForumService.java
@@ -23,4 +23,12 @@ public interface BasicForumService extends Serializable {
ForumThread createThread(String subject);
long countUnreadThreads(List<ForumThread> forumThreadList, User user);
+
+ ForumPost getLastPost(ForumThread forumThread);
+
+ boolean hasAttachments(ForumThread forumThread);
+
+ boolean canDelete(ForumPost forumPost);
+
+ void deletePost(ForumPost post);
}
diff --git a/core/src/main/java/se/su/dsv/scipro/forum/BasicForumServiceImpl.java b/core/src/main/java/se/su/dsv/scipro/forum/BasicForumServiceImpl.java
index c48e3cab91..c74a6fecab 100644
--- a/core/src/main/java/se/su/dsv/scipro/forum/BasicForumServiceImpl.java
+++ b/core/src/main/java/se/su/dsv/scipro/forum/BasicForumServiceImpl.java
@@ -10,6 +10,7 @@ import se.su.dsv.scipro.file.FileService;
import se.su.dsv.scipro.forum.dataobjects.ForumPost;
import se.su.dsv.scipro.forum.dataobjects.ForumPostReadState;
import se.su.dsv.scipro.forum.dataobjects.ForumThread;
+import se.su.dsv.scipro.system.CurrentUser;
import se.su.dsv.scipro.system.User;
public class BasicForumServiceImpl implements BasicForumService {
@@ -19,6 +20,7 @@ public class BasicForumServiceImpl implements BasicForumService {
private final ForumPostReadStateRepository readStateRepository;
private final FileService fileService;
private final EventBus eventBus;
+ private final CurrentUser currentUserProvider;
@Inject
public BasicForumServiceImpl(
@@ -26,13 +28,15 @@ public class BasicForumServiceImpl implements BasicForumService {
final ForumPostReadStateRepository readStateRepository,
AbstractThreadRepository threadRepository,
final FileService fileService,
- final EventBus eventBus
+ final EventBus eventBus,
+ final CurrentUser currentUserProvider
) {
this.postRepository = postRepository;
this.readStateRepository = readStateRepository;
this.threadRepository = threadRepository;
this.fileService = fileService;
this.eventBus = eventBus;
+ this.currentUserProvider = currentUserProvider;
}
@Override
@@ -66,7 +70,7 @@ public class BasicForumServiceImpl implements BasicForumService {
@Override
public boolean isThreadRead(User user, ForumThread forumThread) {
- for (ForumPost post : forumThread.getPosts()) {
+ for (ForumPost post : getPosts(forumThread)) {
if (!getReadState(user, post).isRead()) {
return false;
}
@@ -133,4 +137,56 @@ public class BasicForumServiceImpl implements BasicForumService {
return post;
}
+
+ @Override
+ public ForumPost getLastPost(ForumThread forumThread) {
+ return Collections.max(
+ getPosts(forumThread),
+ Comparator.comparing(ForumPost::getDateCreated).thenComparing(ForumPost::getId)
+ );
+ }
+
+ @Override
+ public boolean hasAttachments(ForumThread forumThread) {
+ for (ForumPost post : getPosts(forumThread)) {
+ if (!post.getAttachments().isEmpty()) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public boolean canDelete(ForumPost forumPost) {
+ ForumPost initialPost = forumPost.getForumThread().getPosts().get(0);
+ if (forumPost.equals(initialPost)) {
+ // The initial post in a thread can never be deleted
+ return false;
+ }
+
+ User user = currentUserProvider.get();
+ // Current user can be null meaning the call came from the system
+ if (user == null) {
+ // Allow the system to delete any post
+ return true;
+ }
+ return Objects.equals(forumPost.getPostedBy(), user);
+ }
+
+ @Override
+ @Transactional
+ public void deletePost(ForumPost post) {
+ if (!canDelete(post)) {
+ throw new PostCantBeDeletedException();
+ }
+ post.setDeleted(true);
+ postRepository.save(post);
+ }
+
+ private static final class PostCantBeDeletedException extends IllegalArgumentException {
+
+ public PostCantBeDeletedException() {
+ super("User is not allowed to delete post");
+ }
+ }
}
diff --git a/core/src/main/java/se/su/dsv/scipro/forum/dataobjects/ForumThread.java b/core/src/main/java/se/su/dsv/scipro/forum/dataobjects/ForumThread.java
index 1c61c2c563..f1933e89c0 100644
--- a/core/src/main/java/se/su/dsv/scipro/forum/dataobjects/ForumThread.java
+++ b/core/src/main/java/se/su/dsv/scipro/forum/dataobjects/ForumThread.java
@@ -116,13 +116,4 @@ public class ForumThread extends LazyDeletableDomainObject {
public User getCreatedBy() {
return getPosts().get(0).getPostedBy();
}
-
- public boolean hasAttachments() {
- for (ForumPost post : posts) {
- if (!post.getAttachments().isEmpty()) {
- return true;
- }
- }
- return false;
- }
}
diff --git a/core/src/test/java/se/su/dsv/scipro/forum/BasicForumServiceImplTest.java b/core/src/test/java/se/su/dsv/scipro/forum/BasicForumServiceImplTest.java
index de6288d6f4..6846f54411 100644
--- a/core/src/test/java/se/su/dsv/scipro/forum/BasicForumServiceImplTest.java
+++ b/core/src/test/java/se/su/dsv/scipro/forum/BasicForumServiceImplTest.java
@@ -121,6 +121,8 @@ public class BasicForumServiceImplTest {
ForumThread forumThread = new ForumThread();
forumThread.addPost(post);
+ when(postRepository.findByThread(forumThread)).thenReturn(List.of(post));
+
when(readStateRepository.find(eq(goodUser), isA(ForumPost.class))).thenReturn(readState);
when(readStateRepository.find(eq(badUser), isA(ForumPost.class))).thenReturn(notReadState);
diff --git a/core/src/test/java/se/su/dsv/scipro/forum/BasicForumServiceIntegrationTest.java b/core/src/test/java/se/su/dsv/scipro/forum/BasicForumServiceIntegrationTest.java
new file mode 100644
index 0000000000..ec5d815af7
--- /dev/null
+++ b/core/src/test/java/se/su/dsv/scipro/forum/BasicForumServiceIntegrationTest.java
@@ -0,0 +1,83 @@
+package se.su.dsv.scipro.forum;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import jakarta.inject.Inject;
+import java.util.Set;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import se.su.dsv.scipro.forum.dataobjects.ForumPost;
+import se.su.dsv.scipro.forum.dataobjects.ForumThread;
+import se.su.dsv.scipro.system.User;
+import se.su.dsv.scipro.test.IntegrationTest;
+
+public class BasicForumServiceIntegrationTest extends IntegrationTest {
+
+ @Inject
+ BasicForumService basicForumService;
+
+ private User op;
+ private User commenter;
+
+ @BeforeEach
+ public void setUp() {
+ User op = User.builder().firstName("Bill").lastName("Gates").emailAddress("bill@example.com").build();
+ this.op = save(op);
+
+ User commenter = User.builder().firstName("Steve").lastName("Jobs").emailAddress("steve@example.com").build();
+ this.commenter = save(commenter);
+ }
+
+ @Test
+ public void can_not_delete_original_post() {
+ ForumThread thread = basicForumService.createThread("Test thread");
+ ForumPost originalPost = basicForumService.createReply(thread, op, "Test post", Set.of());
+
+ setLoggedInAs(op);
+
+ assertFalse(basicForumService.canDelete(originalPost));
+ assertThrows(IllegalArgumentException.class, () -> basicForumService.deletePost(originalPost));
+ }
+
+ @Test
+ public void can_delete_reply_to_original_post() {
+ ForumThread thread = basicForumService.createThread("Test thread");
+ ForumPost originalPost = basicForumService.createReply(thread, op, "Test post", Set.of());
+ ForumPost reply = basicForumService.createReply(thread, commenter, "Test reply", Set.of());
+
+ setLoggedInAs(commenter);
+
+ assertTrue(basicForumService.canDelete(reply));
+ assertDoesNotThrow(() -> basicForumService.deletePost(reply));
+ }
+
+ @Test
+ public void can_not_delete_someone_elses_reply() {
+ ForumThread thread = basicForumService.createThread("Test thread");
+ ForumPost originalPost = basicForumService.createReply(thread, op, "Test post", Set.of());
+ ForumPost reply = basicForumService.createReply(thread, commenter, "Test reply", Set.of());
+
+ setLoggedInAs(op);
+
+ assertFalse(basicForumService.canDelete(reply));
+ assertThrows(IllegalArgumentException.class, () -> basicForumService.deletePost(reply));
+ }
+
+ @Test
+ public void system_can_delete_all_replies() {
+ ForumThread thread = basicForumService.createThread("Test thread");
+ ForumPost originalPost = basicForumService.createReply(thread, op, "Test post", Set.of());
+ ForumPost reply = basicForumService.createReply(thread, commenter, "Test reply", Set.of());
+ ForumPost secondReply = basicForumService.createReply(thread, op, "Test post", Set.of());
+
+ setLoggedInAs(null);
+
+ assertTrue(basicForumService.canDelete(reply));
+ assertDoesNotThrow(() -> basicForumService.deletePost(reply));
+ assertTrue(basicForumService.canDelete(secondReply));
+ assertDoesNotThrow(() -> basicForumService.deletePost(secondReply));
+ }
+}
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 2d52bd0d36..363fc8b80c 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
@@ -1,6 +1,7 @@
package se.su.dsv.scipro.test;
import com.google.common.eventbus.EventBus;
+import jakarta.inject.Inject;
import jakarta.persistence.EntityManager;
import jakarta.persistence.EntityManagerFactory;
import jakarta.persistence.EntityTransaction;
@@ -28,6 +29,7 @@ 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;
+import se.su.dsv.scipro.system.User;
@Testcontainers
public abstract class SpringTest {
@@ -38,6 +40,9 @@ public abstract class SpringTest {
@Container
static MariaDBContainer<?> mariaDBContainer = new MariaDBContainer<>("mariadb:10.11");
+ @Inject
+ private TestUser testUser;
+
private CapturingEventBus capturingEventBus;
@BeforeEach
@@ -64,6 +69,8 @@ public abstract class SpringTest {
annotationConfigApplicationContext.getBeanFactory().registerSingleton("entityManager", this.entityManager);
annotationConfigApplicationContext.refresh();
annotationConfigApplicationContext.getAutowireCapableBeanFactory().autowireBean(this);
+
+ testUser.setUser(null); // default to system
}
@AfterEach
@@ -83,6 +90,10 @@ public abstract class SpringTest {
}
}
+ protected void setLoggedInAs(User user) {
+ this.testUser.setUser(user);
+ }
+
protected List<Object> getPublishedEvents() {
return capturingEventBus.publishedEvents;
}
@@ -108,7 +119,7 @@ public abstract class SpringTest {
@Bean
public CurrentUser currentUser() {
- return () -> null;
+ return new TestUser();
}
@Bean
@@ -129,4 +140,18 @@ public abstract class SpringTest {
super.post(event);
}
}
+
+ private static class TestUser implements CurrentUser {
+
+ private User user;
+
+ @Override
+ public User get() {
+ return user;
+ }
+
+ private void setUser(User user) {
+ this.user = user;
+ }
+ }
}
diff --git a/view/src/main/java/se/su/dsv/scipro/forum/panels/threaded/ForumPostPanel.html b/view/src/main/java/se/su/dsv/scipro/forum/panels/threaded/ForumPostPanel.html
index eba931f634..b09f0405ca 100644
--- a/view/src/main/java/se/su/dsv/scipro/forum/panels/threaded/ForumPostPanel.html
+++ b/view/src/main/java/se/su/dsv/scipro/forum/panels/threaded/ForumPostPanel.html
@@ -7,9 +7,10 @@
<body>
<wicket:panel>
<div class="messageWrap">
- <div class="forumBlueBackground">
+ <div class="forumBlueBackground d-flex justify-content-between">
<!-- DATE ROW-->
- <wicket:container wicket:id="dateCreated"/>
+ <span wicket:id="dateCreated"></span>
+ <button wicket:id="delete" class="btn btn-sm btn-outline-danger ms-auto">Delete</button>
</div>
<div class="forumGrayBackground">
<div class="vertAlign">
diff --git a/view/src/main/java/se/su/dsv/scipro/forum/panels/threaded/ForumPostPanel.java b/view/src/main/java/se/su/dsv/scipro/forum/panels/threaded/ForumPostPanel.java
index 004d99561a..225240d8e1 100644
--- a/view/src/main/java/se/su/dsv/scipro/forum/panels/threaded/ForumPostPanel.java
+++ b/view/src/main/java/se/su/dsv/scipro/forum/panels/threaded/ForumPostPanel.java
@@ -1,7 +1,9 @@
package se.su.dsv.scipro.forum.panels.threaded;
+import jakarta.inject.Inject;
import org.apache.wicket.Component;
import org.apache.wicket.markup.html.WebMarkupContainer;
+import org.apache.wicket.markup.html.link.Link;
import org.apache.wicket.markup.html.panel.Panel;
import org.apache.wicket.model.IModel;
import org.apache.wicket.model.LambdaModel;
@@ -11,9 +13,11 @@ import se.su.dsv.scipro.components.ListAdapterModel;
import se.su.dsv.scipro.components.SmarterLinkMultiLineLabel;
import se.su.dsv.scipro.data.enums.DateStyle;
import se.su.dsv.scipro.file.FileReference;
+import se.su.dsv.scipro.forum.BasicForumService;
import se.su.dsv.scipro.forum.dataobjects.ForumPost;
import se.su.dsv.scipro.profile.UserLinkPanel;
import se.su.dsv.scipro.repository.panels.ViewAttachmentPanel;
+import se.su.dsv.scipro.session.SciProSession;
public class ForumPostPanel extends Panel {
@@ -22,6 +26,9 @@ public class ForumPostPanel extends Panel {
public static final String CONTENT = "content";
public static final String ATTACHMENT = "attachment";
+ @Inject
+ private BasicForumService basicForumService;
+
public ForumPostPanel(String id, final IModel<ForumPost> model) {
super(id);
add(new UserLinkPanel(POSTED_BY, LambdaModel.of(model, ForumPost::getPostedBy, ForumPost::setPostedBy)));
@@ -62,5 +69,28 @@ public class ForumPostPanel extends Panel {
}
}
);
+
+ add(
+ new Link<>("delete", model) {
+ @Override
+ public void onClick() {
+ ForumPost post = getModelObject();
+ basicForumService.deletePost(post);
+ onPostDeleted();
+ }
+
+ @Override
+ protected void onConfigure() {
+ super.onConfigure();
+ setVisible(allowDeletion() && basicForumService.canDelete(getModelObject()));
+ }
+ }
+ );
}
+
+ protected boolean allowDeletion() {
+ return false;
+ }
+
+ protected void onPostDeleted() {}
}
diff --git a/view/src/main/java/se/su/dsv/scipro/forum/panels/threaded/ThreadsOverviewPanel.java b/view/src/main/java/se/su/dsv/scipro/forum/panels/threaded/ThreadsOverviewPanel.java
index 02a9d00bd5..d0e7e1c55d 100644
--- a/view/src/main/java/se/su/dsv/scipro/forum/panels/threaded/ThreadsOverviewPanel.java
+++ b/view/src/main/java/se/su/dsv/scipro/forum/panels/threaded/ThreadsOverviewPanel.java
@@ -1,8 +1,7 @@
package se.su.dsv.scipro.forum.panels.threaded;
+import jakarta.inject.Inject;
import java.io.Serializable;
-import java.util.Collections;
-import java.util.Comparator;
import java.util.List;
import org.apache.wicket.markup.html.WebMarkupContainer;
import org.apache.wicket.markup.html.basic.Label;
@@ -15,6 +14,7 @@ import org.apache.wicket.model.LambdaModel;
import org.apache.wicket.model.LoadableDetachableModel;
import se.su.dsv.scipro.components.DateLabel;
import se.su.dsv.scipro.data.enums.DateStyle;
+import se.su.dsv.scipro.forum.BasicForumService;
import se.su.dsv.scipro.forum.Discussable;
import se.su.dsv.scipro.forum.dataobjects.ForumPost;
import se.su.dsv.scipro.forum.dataobjects.ForumThread;
@@ -23,6 +23,9 @@ import se.su.dsv.scipro.system.User;
public class ThreadsOverviewPanel<A> extends Panel {
+ @Inject
+ private BasicForumService basicForumService;
+
public ThreadsOverviewPanel(
final String id,
final IModel<List<A>> model,
@@ -41,7 +44,7 @@ public class ThreadsOverviewPanel<A> extends Panel {
@Override
protected void onConfigure() {
super.onConfigure();
- setVisibilityAllowed(discussion.getObject().hasAttachments());
+ setVisibilityAllowed(basicForumService.hasAttachments(discussion.getObject()));
}
}
);
@@ -80,7 +83,7 @@ public class ThreadsOverviewPanel<A> extends Panel {
BookmarkablePageLink<Void> newThreadLink(String id, IModel<A> thread);
}
- private static class LastPostColumn extends WebMarkupContainer {
+ private class LastPostColumn extends WebMarkupContainer {
public LastPostColumn(String id, final IModel<ForumThread> model) {
super(id);
@@ -110,10 +113,7 @@ public class ThreadsOverviewPanel<A> extends Panel {
return new LoadableDetachableModel<>() {
@Override
protected ForumPost load() {
- return Collections.max(
- model.getObject().getPosts(),
- Comparator.comparing(ForumPost::getDateCreated).thenComparing(ForumPost::getId)
- );
+ return basicForumService.getLastPost(model.getObject());
}
};
}
diff --git a/view/src/main/java/se/su/dsv/scipro/forum/panels/threaded/ViewForumThreadPanel.java b/view/src/main/java/se/su/dsv/scipro/forum/panels/threaded/ViewForumThreadPanel.java
index 9c62f77122..06b89d7e14 100644
--- a/view/src/main/java/se/su/dsv/scipro/forum/panels/threaded/ViewForumThreadPanel.java
+++ b/view/src/main/java/se/su/dsv/scipro/forum/panels/threaded/ViewForumThreadPanel.java
@@ -58,7 +58,21 @@ public class ViewForumThreadPanel<A> extends GenericPanel<A> {
new ListView<>(POST_LIST, new PostProvider()) {
@Override
protected void populateItem(ListItem<ForumPost> item) {
- item.add(new ForumPostPanel(POST, item.getModel()));
+ ListView<ForumPost> listView = this;
+ item.add(
+ new ForumPostPanel(POST, item.getModel()) {
+ @Override
+ protected boolean allowDeletion() {
+ return true;
+ }
+
+ @Override
+ protected void onPostDeleted() {
+ // Refresh the list of posts
+ listView.detach();
+ }
+ }
+ );
}
}
);
diff --git a/view/src/test/java/se/su/dsv/scipro/forum/panels/threaded/ThreadsOverviewPanelTest.java b/view/src/test/java/se/su/dsv/scipro/forum/panels/threaded/ThreadsOverviewPanelTest.java
index 8c99db6c4f..614f92e7cd 100644
--- a/view/src/test/java/se/su/dsv/scipro/forum/panels/threaded/ThreadsOverviewPanelTest.java
+++ b/view/src/test/java/se/su/dsv/scipro/forum/panels/threaded/ThreadsOverviewPanelTest.java
@@ -9,6 +9,7 @@ import org.apache.wicket.model.Model;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Mockito;
import org.mockito.junit.jupiter.MockitoExtension;
import se.su.dsv.scipro.SciProTest;
import se.su.dsv.scipro.forum.Discussable;
@@ -20,11 +21,13 @@ import se.su.dsv.scipro.system.User;
public class ThreadsOverviewPanelTest extends SciProTest {
private List<ForumThread> threads;
+ private ForumPost post;
@BeforeEach
public void setUp() throws Exception {
ForumThread forumThread = createThread();
threads = Arrays.asList(forumThread);
+ Mockito.when(basicForumService.getLastPost(forumThread)).thenReturn(post);
}
@Test
@@ -54,7 +57,7 @@ public class ThreadsOverviewPanelTest extends SciProTest {
private ForumThread createThread() {
User bob = User.builder().firstName("Bob").lastName("the Builder").emailAddress("bob@building.com").build();
- ForumPost post = new ForumPost();
+ post = new ForumPost();
post.setPostedBy(bob);
ForumThread groupForumThread = new ForumThread();
groupForumThread.addPost(post);
From 2ac30fa98041fcbc29a31e9d93132c31bd7358c9 Mon Sep 17 00:00:00 2001
From: Andreas Svanberg <andreass@dsv.su.se>
Date: Tue, 18 Mar 2025 09:12:41 +0100
Subject: [PATCH 6/9] Add Checkstyle checking during Maven build (#138)
So far no rules are activated and it just puts the infrastructure in place.
Rules can be added in separately after discussion among the developers,
along with fixing any violations of the rules.
Co-authored-by: Nico Athanassiadis <nico@dsv.su.se>
Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/138
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>
---
Dockerfile | 1 +
checkstyle.xml | 7 +++++++
pom.xml | 22 ++++++++++++++++++++++
3 files changed, 30 insertions(+)
create mode 100644 checkstyle.xml
diff --git a/Dockerfile b/Dockerfile
index acf3c889a3..71ff8ba3a5 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -24,6 +24,7 @@ COPY test-data/src/ test-data/src/
RUN ./mvnw package \
--define skipTests \
--activate-profiles branch,DEV \
+ --define checkstyle.skip=true \
--define skip.npm \
--define skip.installnodenpm
diff --git a/checkstyle.xml b/checkstyle.xml
new file mode 100644
index 0000000000..bd24b31e31
--- /dev/null
+++ b/checkstyle.xml
@@ -0,0 +1,7 @@
+<?xml version="1.0"?>
+<!DOCTYPE module PUBLIC
+ "-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
+ "https://checkstyle.org/dtds/configuration_1_3.dtd">
+
+<module name="Checker">
+</module>
diff --git a/pom.xml b/pom.xml
index 3473242939..8567faf79d 100755
--- a/pom.xml
+++ b/pom.xml
@@ -211,6 +211,11 @@
<artifactId>maven-war-plugin</artifactId>
<version>3.4.0</version>
</plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-checkstyle-plugin</artifactId>
+ <version>3.6.0</version>
+ </plugin>
</plugins>
</pluginManagement>
<plugins>
@@ -312,6 +317,23 @@
</execution>
</executions>
</plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-checkstyle-plugin</artifactId>
+ <configuration>
+ <configLocation>checkstyle.xml</configLocation>
+ <failOnViolation>true</failOnViolation>
+ </configuration>
+ <executions>
+ <execution>
+ <id>validate</id>
+ <phase>process-sources</phase>
+ <goals>
+ <goal>check</goal>
+ </goals>
+ </execution>
+ </executions>
+ </plugin>
</plugins>
</build>
</project>
\ No newline at end of file
From 9fa699ed8371960e9f2e5e57cb0378a398b95522 Mon Sep 17 00:00:00 2001
From: Andreas Svanberg <andreass@dsv.su.se>
Date: Thu, 20 Mar 2025 06:44:13 +0100
Subject: [PATCH 7/9] Upgrade Spring Boot (#140)
The new version has upgraded `json-smart` so the override is no longer necessary.
## How to test
1. Log in and click around as different users
2. Enable Daisy integration (both `DEV` and `DAISY-INTEGRATION` Maven profiles)
3. Go to "Admin / Users / Import" and import someone (verify JSON parsing)
Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/140
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 | 22 +---------------------
1 file changed, 1 insertion(+), 21 deletions(-)
diff --git a/pom.xml b/pom.xml
index 8567faf79d..617362a163 100755
--- a/pom.xml
+++ b/pom.xml
@@ -28,12 +28,7 @@
<querydsl.version>5.0.0</querydsl.version>
<poi.version>5.4.0</poi.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>
+ <spring.boot.version>3.4.3</spring.boot.version>
<springdoc.openapi.version>2.8.3</springdoc.openapi.version>
<!-- Database stuff -->
@@ -115,21 +110,6 @@
<version>32.0.1-jre</version>
</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>
-
<dependency>
<groupId>org.apache.poi</groupId>
<artifactId>poi</artifactId>
From e95421b8f2f6cb232f8ce735d9be963a07e0be60 Mon Sep 17 00:00:00 2001
From: Andreas Svanberg <andreass@dsv.su.se>
Date: Tue, 25 Mar 2025 08:45:25 +0100
Subject: [PATCH 8/9] Use OAuth 2.0 Token Introspection during log in (#141)
Currently, it uses an endpoint similar to OpenID Connect UserInfo but with some differences. The endpoint does not require the "openid" scope for example. There is an ongoing effort to replace the OAuth 2.0 authorization server with a more standard compliant one which would break the endpoint (since it would require the "openid" scope). It is currently not possible to request the "openid" scope to future-proof since Spring would act differently if that scope is present and assume full OpenID Connect. That leads to requiring an id token to have been issued which the current authorization server does not do.
To get around this the implementation is changed to use a standard compliant Token Introspection endpoint to get access to the subject of the access token (which is the only part that's necessary right now). Since the endpoint is standard compliant it will work with any future authorization server.
It may be necessary to run `docker compose up --build` to get the latest version of the Toker containers.
Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/141
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>
---
compose-branch-deploy.yaml | 2 +-
...enIntrospectionRequestEntityConverter.java | 42 +++++++++++++++++++
.../dsv/scipro/war/WicketConfiguration.java | 16 +++++++
war/src/main/resources/application.properties | 2 +-
4 files changed, 60 insertions(+), 2 deletions(-)
create mode 100644 war/src/main/java/se/su/dsv/scipro/war/TokenIntrospectionRequestEntityConverter.java
diff --git a/compose-branch-deploy.yaml b/compose-branch-deploy.yaml
index 05ba4bd181..68694cc0b6 100644
--- a/compose-branch-deploy.yaml
+++ b/compose-branch-deploy.yaml
@@ -15,7 +15,7 @@ services:
- JDBC_DATABASE_PASSWORD=scipro
- OAUTH2_AUTHORIZATION_URI=https://oauth2-${VHOST}/authorize
- OAUTH2_TOKEN_URI=https://oauth2-${VHOST}/exchange
- - OAUTH2_USER_INFO_URI=https://oauth2-${VHOST}/verify
+ - OAUTH2_USER_INFO_URI=https://oauth2-${VHOST}/introspect
- OAUTH2_CLIENT_ID=scipro_client
- OAUTH2_CLIENT_SECRET=scipro_secret
- OAUTH2_RESOURCE_SERVER_ID=scipro_api_client
diff --git a/war/src/main/java/se/su/dsv/scipro/war/TokenIntrospectionRequestEntityConverter.java b/war/src/main/java/se/su/dsv/scipro/war/TokenIntrospectionRequestEntityConverter.java
new file mode 100644
index 0000000000..7b5cfa9e96
--- /dev/null
+++ b/war/src/main/java/se/su/dsv/scipro/war/TokenIntrospectionRequestEntityConverter.java
@@ -0,0 +1,42 @@
+package se.su.dsv.scipro.war;
+
+import java.net.URI;
+import java.util.Collections;
+import org.springframework.core.convert.converter.Converter;
+import org.springframework.http.HttpHeaders;
+import org.springframework.http.HttpMethod;
+import org.springframework.http.MediaType;
+import org.springframework.http.RequestEntity;
+import org.springframework.security.oauth2.client.registration.ClientRegistration;
+import org.springframework.security.oauth2.client.userinfo.OAuth2UserRequest;
+import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames;
+import org.springframework.util.LinkedMultiValueMap;
+import org.springframework.util.MultiValueMap;
+import org.springframework.web.util.UriComponentsBuilder;
+
+public class TokenIntrospectionRequestEntityConverter implements Converter<OAuth2UserRequest, RequestEntity<?>> {
+
+ private static final MediaType FORM_URL_ENCODED = MediaType.valueOf(
+ MediaType.APPLICATION_FORM_URLENCODED_VALUE + ";charset=UTF-8"
+ );
+
+ @Override
+ public RequestEntity<?> convert(OAuth2UserRequest userRequest) {
+ ClientRegistration clientRegistration = userRequest.getClientRegistration();
+
+ URI uri = UriComponentsBuilder.fromUriString(
+ clientRegistration.getProviderDetails().getUserInfoEndpoint().getUri()
+ )
+ .build()
+ .toUri();
+
+ HttpHeaders headers = new HttpHeaders();
+ headers.setBasicAuth(clientRegistration.getClientId(), clientRegistration.getClientSecret());
+ headers.setAccept(Collections.singletonList(MediaType.ALL));
+ headers.setContentType(FORM_URL_ENCODED);
+
+ MultiValueMap<String, String> formParameters = new LinkedMultiValueMap<>();
+ formParameters.add(OAuth2ParameterNames.TOKEN, userRequest.getAccessToken().getTokenValue());
+ return new RequestEntity<>(formParameters, headers, HttpMethod.POST, uri);
+ }
+}
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 90b0b70a61..064728fcbc 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
@@ -14,6 +14,7 @@ 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.oauth2.client.userinfo.DefaultOAuth2UserService;
import org.springframework.security.web.SecurityFilterChain;
import se.su.dsv.scipro.SciProApplication;
import se.su.dsv.scipro.crosscutting.ForwardPhase2Feedback;
@@ -67,6 +68,21 @@ public class WicketConfiguration {
return http.build();
}
+ // Stop gap measure to switch to Token Introspection instead of OIDC UserInfo
+ // endpoint. This is necessary because the UserInfo endpoint will in soon require
+ // the "openid" scope, which is not granted to our clients. Unfortunately we can't
+ // request the scope because that makes Spring require an id token in the token
+ // exchange which is not granted at the moment.
+ //
+ // Once a new authorization server is in place we can remove this bean and use
+ // straight up id tokens with "openid" scope.
+ @Bean
+ public DefaultOAuth2UserService defaultOAuth2UserService() {
+ DefaultOAuth2UserService defaultOAuth2UserService = new DefaultOAuth2UserService();
+ defaultOAuth2UserService.setRequestEntityConverter(new TokenIntrospectionRequestEntityConverter());
+ return defaultOAuth2UserService;
+ }
+
@Bean
public CurrentUserFromSpringSecurity currentUserFromSpringSecurity(
UserService userService,
diff --git a/war/src/main/resources/application.properties b/war/src/main/resources/application.properties
index 0df6b375e6..7b0b3f2105 100644
--- a/war/src/main/resources/application.properties
+++ b/war/src/main/resources/application.properties
@@ -22,7 +22,7 @@ spring.security.oauth2.resourceserver.opaquetoken.client-secret=${OAUTH2_RESOURC
spring.security.oauth2.resourceserver.opaquetoken.introspection-uri=${OAUTH2_RESOURCE_SERVER_INTROSPECTION_URI:http://localhost:59733/introspect}
# Log in via local OAuth 2 authorization server
-spring.security.oauth2.client.provider.docker.user-info-uri=${OAUTH2_USER_INFO_URI:http://localhost:59734/verify}
+spring.security.oauth2.client.provider.docker.user-info-uri=${OAUTH2_USER_INFO_URI:http://localhost:59734/introspect}
spring.security.oauth2.client.provider.docker.user-name-attribute=sub
spring.security.oauth2.client.provider.docker.token-uri=${OAUTH2_TOKEN_URI:http://localhost:59734/exchange}
spring.security.oauth2.client.provider.docker.authorization-uri=${OAUTH2_AUTHORIZATION_URI:http://localhost:59734/authorize}
From d2e5043c959504faad7fd307e9312e89c3c1a96d Mon Sep 17 00:00:00 2001
From: Andreas Svanberg <andreass@dsv.su.se>
Date: Fri, 28 Mar 2025 07:18:16 +0100
Subject: [PATCH 9/9] Fix CVE-2025-22228 (#143)
See https://spring.io/security/cve-2025-22228
Fixes #142
Reviewed-on: https://gitea.dsv.su.se/DMC/scipro/pulls/143
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 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/pom.xml b/pom.xml
index 617362a163..4d7f8a5d4a 100755
--- a/pom.xml
+++ b/pom.xml
@@ -28,7 +28,7 @@
<querydsl.version>5.0.0</querydsl.version>
<poi.version>5.4.0</poi.version>
- <spring.boot.version>3.4.3</spring.boot.version>
+ <spring.boot.version>3.4.4</spring.boot.version>
<springdoc.openapi.version>2.8.3</springdoc.openapi.version>
<!-- Database stuff -->