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: #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>
Fixes#43 by introducing [Prettier](https://prettier.io/).
Prettier is an extremely opinionated formatter. It will reformat every single line according to its style. There are virtually no configuration options so there can be no discussion about formatting rules.
There are two parameters that are configurable; indent length and line length. Indent length has been set to 4 because that's the Java standard.
Line length defaults to 80 but has been increased to 100. The rational for this is that Prettier was created for JavaScript which is much less verbose than Java. Not only does every Java line start with 8 spaces of indentation vs. JavaScripts 0 or 2, it also has types wile JavaScript does not and uses `const` for variable declarations. Compare the two below examples as well as an actual example from the source code that is too long for the default 80 characters. I have no problem dropping down to the default 80 if that is preferred I just felt that with the average length of a line of Java code being pretty long, excessive wrapping would reduce readability.
```javascript
const attributes = {
...
};
```
```java
Map<String, String> attributes = Map.of(
...
);
```
Or the following real code which is 97 characters long.
```java
Set<ProjectParticipant> contributors = daisyAPI.getContributors(project.getIdentifier());
```
Reviewed-on: #44
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>
Daisy's API does not support the function "change supervisor" and only what comes down to SQL INSERT and DELETE on a specific table. If the removal of the previous supervisor(s) failed the new one was still added. This change makes it so that if the removal of any (there is no limit in the API) current supervisor fails it will not attempt to add the new supervisor.
Fixes#39
## How to test
1. Configure the Daisy integration in SciPro
2. Enable it (Maven profile)
3. The only currently known way to make removing a supervisor fail is to use a project where they've sent the thesis to the examiner
4. Find one such project in the test server for Daisy (or send one for examination from SciPro)
5. Change the supervisor of the project in SciPro
6. Run the job "Export projects to daisy" (System / Maintenance)
7. See that it logs the failure and that no duplicate supervisors were added to Daisy
Reviewed-on: #26
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>
With the migration to Spring it is much stricter in requiring @Transactional for all methods that write to the database.
Fixes#22
Co-authored-by: Tom Zhao <tom.zhao@dsv.su.se>
Reviewed-on: #23
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>
This is one requirement in bringing #15 to reality.
Currently there are some 450 migration scripts that have been added over the past 11 years. Unfortunately some of these migration scripts have some defects. Either from the fact that they are very old and from another database engine (MySQL vs currently MariaDB), make assumptions about the database name, or its contents. Due to these defects trying to bring an empty schema up-to-date by running all migrations will fail with [372](ff4c5b58b4/core/src/main/resources/db/migration/V372__update_and_insert_grading_criterion_template_master.sql) being the main blocker.
If it is not possible to bring an empty schema up-to-date it is a major hindrance to the plan of automatically deploying test servers for every pull request (#15). These changes makes it possible to bring an empty schema up to the latest version by squashing all migration scripts to a single new baseline with the necessary fixes to work on an empty schema.
There is a downside with the way it accomplishes this, it requires any non-empty schema to already be at version [392.2](ff4c5b58b4/core/src/main/resources/db/migration/V392_2__reflection_comment_by_supervisor.sql). [Flyway](https://www.red-gate.com/products/flyway/), the product we use for database migrations, does not support new baseline scripts in the free version, only in the paid edition. To get around this, Flyway is tricked into thinking the database has never used Flyway before by changing which database table stores the information about applied migrations. This is the reason the database has to be at the latest (392.2) version before deploying the new version of SciPro that include this change, because Flyway will have no way to see which of the old migrations have been applied.
An alternative would be to fix the old migrations so they would work on an empty schema. However, since every migration script is checksummed to see that the applied version is the correct one every database would have to be ["repaired"](https://documentation.red-gate.com/fd/repair-184127461.html) to update its checksums. This choice was not taken for two reasons:
* It would require manual work in the database before deploying the new version of SciPro with the fixed migrations, similar to the requirement to first deploy the version of SciPro that includes the 392.2 migration.
* Running all the migrations taken a lot of time, especially the new [391](ff4c5b58b4/core/src/main/resources/db/migration/V391__harmonize_table_attribute_name.sql). Squashing all migrations avoid this and makes spinning up new databases very quick
## How to test with an existing schema
1. Deploy commit [ff4c5b58b4](ff4c5b58b4) (current `develop` branch as of 2024-11-22)
2. Start the system to apply migrations up to and including 392.2
3. Switch to this branch
4. Start the system and see that the database will be considered baselined at version 2
5. Click around in the system and see that it still works
## How to test with an empty schema
1. Empty your database schema
2. Switch to this branch
3. Deploy the system
4. See that it migrates the schema and creates all the necessary tables
5. Log in as `admin@example.com` that is created by the `DataInitializer`
Reviewed-on: #24
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>
Changes made to the grading template UI
Some of the improvements include:
- When editing a template, and the user makes a change the user will be informed/alerted that the template has changed.
- When editing a template the user will now see a project title that shows what project type that template has.
- Default value of points required to pass has been changed from 0 -> 1
- Requirements to points will be added by default when adding a new criteria.
- The create button is now hidden until the user has chosen a grading template project type from the drop down menu.
- Max available points are now updated as point requirements are added to the criterion.
Reviewed-on: #21
Reviewed-by: Andreas Svanberg <andreass@dsv.su.se>
Co-authored-by: Nico Athanassiadis <nico@dsv.su.se>
Co-committed-by: Nico Athanassiadis <nico@dsv.su.se>
Replaces #12
Fixes card 3213 and 3412
There are minimum requirements for the reflection document submitted by authors at the end of the thesis process. Before now there was no way to handle the case when the reflection did not meet these minimum requirements.
This change makes it possible in two ways;
1. The supervisor can request improvements to be made requiring the author to re-submit a new reflection inside SciPro
2. The supervisor can directly edit the reflection themselves if it has been submitted out-of-band or for any other reason
Co-authored-by: Nico Athanassiadis <nico@dsv.su.se>
Reviewed-on: #13
Reviewed-by: Nico Athanassiadis <nico@dsv.su.se>
A few missing `@Bean` definitions were discovered.
Spring also has a much stricter requirement about requiring `@Transactional` for every database change.
## How to test `@Bean GroupFacadeImpl`
1. Log in as a supervisor
2. Go to "My groups" and create a group
3. Go back to "My groups" and try to open the group
## How to test `@Bean IdeaFacadae`
1. Create an application period that is open ("Admin" / "Match" / "Application periods")
2. Log in as an author
3. Go to "Ideas" / "My ideas"
4. Click "Select from available ideas" in the period created in step 1
## How to test missing `@Transactional`
1. Log in as a user with notifications (or generate some by for example writing in the forum)
2. Go to "Notifications" in the top right
3. Click on any notification subject to navigate to it
## How to test crash while trying to schedule final seminar
1. Log in as a supervisor
2. Open a project and attempt to schedule a final seminar
## How to test crash while trying to upload final thesis as supervisor
1. Log in as supervisor
2. Note down the supervisors username
3. Open a project that has had a final seminar
4. Go to the "Finishing up" tab
5. Submit the supervisors username as the custom principal
6. Try to upload a final thesis
## How to test removal of approved final thesis
1. Log in as supervisor
2. Note down the supervisors username
3. Open a project that has had a final seminar
4. Go to the "Finishing up" tab
5. Submit the supervisors username as the custom principal
6. Upload a final thesis
7. Approve the final thesis
8. Remove the approval
## How to test crash while trying to schedule first meeting
1. Log in as supervisor
2. Open a project
3. Go to the "First meeting tab"
4. Try to submit
## How to test crash while trying to unselect an idea as an author
1. Create an application period that is open ("Admin" / "Match" / "Application periods")
2. Log in as an author
3. Go to "Ideas" / "My ideas"
4. Click "Select from available ideas" in the period created in step 1
5. Select any available supervisor idea
6. Go back to "Ideas" / "My ideas"
7. Open the selected idea
8. Hit unselect at the bottom
## How to test crash while trying to toggle milestone
1. Log in as supervisor
2. Open any project
3. Go to "Milestones" tab
4. Attempt to toggle both individual and project milestones
## How to test crash while trying to get user's note
1. Find a user without a row in the `note` table
2. Log in as that user
3. Click "My notes" in the top right
Reviewed-on: #20
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>
* number of rows in textarea is reduced from 20 to 7.
* made small reformatting of ProjectNoteColumn
Reviewed-on: #19
Reviewed-by: ansv7779 <andreass@dsv.su.se>
Co-authored-by: Tom Zhao <tom.zhao@dsv.su.se>
Co-committed-by: Tom Zhao <tom.zhao@dsv.su.se>
When you tried to edit a grading template in "Project management" -> "Grading Templates" the criterion could be shown in an order not reflecting the sortOrder of the criterion.
The reason was that we only got the arbitrary order depending on the insertion order in the database.
Changed the Collection to a List, it is now an ArrayList. Also added @OrderBy("sortOrder ASC") to actually sort by the order of the criterion thus ensuring visual order is intact.
Co-authored-by: Tom Zhao <tom.zhao@dsv.su.se>
Reviewed-on: #18
Reviewed-by: Tom Zhao <tom.zhao@dsv.su.se>
Co-authored-by: Nico Athanassiadis <nico@dsv.su.se>
Co-committed-by: Nico Athanassiadis <nico@dsv.su.se>
By default, Tomcat will use a cookie to track the session. However, if there is no cookie sent by the browser it will append the session id to the URL. The way it does this is by adding a ";jsessionid=..." to the end. This is not a problem in itself, but it can enable session hijacking if the URL is shared and ";" is a blocked character by the default Spring Security configuration (see StrictHttpFirewall).
So what happens is a user navigates to SciPro. No session cookie is sent since this is the first request. SciPro sees that the user is not authenticated and redirects the user to the login page. When SciPro checks for authentication it checks the session which will instruct Tomcat to create a session. Since Tomcat sees no cookie it will append the session id to the redirect URL to try and track the session. After the user has logged in they are redirected back to SciPro with the session id in the URL which is then blocked by Spring's StrictHttpFirewall.
To avoid this, we can set the tracking mode to *only* COOKIE.
An alternative solution is to tell Spring to allow ";" in the URL but there seems to be good reason as to why it is blocked, see the Javadoc linked below.
ab93541926/web/src/main/java/org/springframework/security/web/firewall/StrictHttpFirewall.java (L202)
## How to test
1. Open a new private browsing window (to make sure there are no cookies).
2. Go to http://localhost:8080 (or wherever you have SciPro running) while on the `develop` branch
3. See that you're stuck on a blank white page with a ";jsessionid=..." in the URL with a 401 response
4. Remove the ";jsessionid=..." part and you'll be logged in to SciPro
5. Switch to this branch and try and see that you'll be logged in immediately
Reviewed-on: #16
Reviewed-by: niat8586 <nico@dsv.su.se>
Co-authored-by: Andreas Svanberg <andreass@dsv.su.se>
Co-committed-by: Andreas Svanberg <andreass@dsv.su.se>
The titles entered are most often the same as the project title. The project title however has no length limit. When such a project tries to upload their final thesis the system will crash because the accepted long project title is rejected as the final thesis title.
This change removes the length limit on final thesis titles to bring them to parity with the project title.
See OTRS ticket [202411011026](https://otrs.dsv.su.se/otrs/index.pl?Action=AgentTicketZoom;TicketID=389771#1413199)
This change has already been done manually in the production system to resolve the above ticket.
## How to test
Submit a final thesis using a title longer than 255 characters and see that is accepted.
Reviewed-on: #17
Reviewed-by: niat8586 <nico@dsv.su.se>
Co-authored-by: Andreas Svanberg <andreass@dsv.su.se>
Co-committed-by: Andreas Svanberg <andreass@dsv.su.se>
SciPro will have to provide information to the upcoming student portal. Wicket does not have the ability to serve JSON in the usual REST way and is only able to serve HTML. The most common way to write JSON over HTTP API:s in Java is using Spring Web, but currently SciPro uses Guice for dependency injection rather than Spring which makes adding Spring Web a bit more tricky.
This pull request attempts to solve this by doing the following;
* Replacing Guice with Spring
* Adding a new API module that uses Spring Web
* Turning the entire system into a standard Spring Boot web application
The hope is that these changes will bring the following benefits;
* Harmonize our web stack (Daisy uses Spring and the new lecture hall system is full Spring Boot)
* Enable easy development of a traditional JSON over HTTP API
* Ease future recruitment by using the most common Java web frameworks
Reviewed-on: #5
Reviewed-by: niat8586 <nico@dsv.su.se>
Co-authored-by: Andreas Svanberg <andreass@dsv.su.se>
Co-committed-by: Andreas Svanberg <andreass@dsv.su.se>
Adds an indicator to every commit and pull request that shows if every test passes.
Uses [Maven Wrapper](https://maven.apache.org/wrapper/) instead of relying on a system wide available `mvn` command. This ensures a stable build environment with a set version of Maven.
Co-authored-by: niat8586 <nico@dsv.su.se>
Reviewed-on: #9
Reviewed-by: niat8586 <nico@dsv.su.se>
Co-authored-by: Andreas Svanberg <andreass@dsv.su.se>
Co-committed-by: Andreas Svanberg <andreass@dsv.su.se>
# Manage grading report templates
Every once in a while the criteria used to asses theses and individual performance changes. When this happens the criteria in SciPro needs to be updated.
Up until now there has not been a GUI to do this and it has instead been done manually, sometimes using database migrations. It was also time-sensitive since there was only ever one template (per project type) in SciPro such that the change had to happen at the correct time when the new template was to take effect.
There was also the question of how to calculate the grade the author should receive based on the point sum from the supervisors assessment of all the criteria and these limits were hard-coded in the system.
These changes address all of the above;
* There can be multiple templates per type.
* Each template has a period during which it is are current.
* Each template has defined the conversion of point sums to grades.
The periods can not overlap and a template remains current from its start date until another template's start date is reached. There can not be two templates (within a project type) that become valid on the same date.
## How to test
1. Log in as administrator
2. Go to "Project management"
3. Go to "Grading templates"
4. Create/edit new templates
5. View current template
Card 3482
Co-authored-by: niat8586 <nico@dsv.su.se>
Reviewed-on: #14
Reviewed-by: niat8586 <nico@dsv.su.se>
Co-authored-by: Andreas Svanberg <andreass@dsv.su.se>
Co-committed-by: Andreas Svanberg <andreass@dsv.su.se>
These classes were copied from Spring Data a long time ago. Their existence on the class path confuses Spring Boot into thinking we use Spring Data JPA which we do not.
changed the wicket version due to security scan
Reviewed-on: #10
Reviewed-by: niat8586 <nico@dsv.su.se>
Co-authored-by: wwest <wayne@dsv.su.se>
Co-committed-by: wwest <wayne@dsv.su.se>
There is a need among supervisors to maintain some work notes for each project. Where they are in the process, what students need to do, and other such things.
This PR gives the supervisors the ability to write a note for each project on their overview page.
Card 3399
Reviewed-on: DSV/scipro#8
Reviewed-by: niat8586 <nico@dsv.su.se>
Their project overview page now include a column indicating if any thread in that forum has an unread post and it also acts as a quick link directly to the forum.
Fixes#2
Card 3399
Reviewed-on: DSV/scipro#3
Reviewed-by: niat8586 <nico@dsv.su.se>
Co-authored-by: Andreas Svanberg <andreass@dsv.su.se>
Co-committed-by: Andreas Svanberg <andreass@dsv.su.se>
Administrators want to inform the reviewers what dates are important during the thesis writing process.
They can now configure an information URL for each type of project under "Admin / System / Type settings" that is shown to the reviewers as they review projects.
Administrators want to inform the supervisors how the reviewers are assigned and what dates are important during the thesis writing process.
They can now configure an information URL for each type of project under "Admin / System / Type settings" that is shown to the supervisors as they enter the review process.
There has always been a minimum and maximum limit on the number of authors per project type, it was just never configurable. Made them configurable while working on the panel.
Targets set by administrators are based on how many projects the reviewer gets assigned. If there are multiple reviews on a single project due to rejecting earlier versions the reviewer does not get more time.
Can not fetch the settings from the authenticator since that is ran on a HTTP request dispatching thread where no UnitOfWork has started.It is not possible to fetch the credentials in the constructor since then it would not be possible to change the settings while the system is running. Instead recreate the client with prefetched credentials on every request.
What happens in there are simultaneous requests to open a checklist.
Both threads will read the checklist from the database, and since it
is versioned they will both get version N.
Then both threads will try to update the last open date and save the
checklist. When saving a versioned object a SQL query of the form
UPDATE checklist SET version = N+1 WHERE id = ? AND version = N
is generated, the first such query will succeed but the second one
will not update any row which Hibernate treats as an optimistic
locking error.
To work around this we use a global lock to ensure that only one
thread can update the checklist at a time. We can't use the
EntityManagers lock method since that too will cause an optimistic
locking failure. This is an unfortunate global lock that will affect
everyone even if they open different checklist, but it is such an
underused feature that it should not be a problem.
