7 Commits

Author SHA1 Message Date
323d6fc61e Automate deployment of pull requests (#15)
All checks were successful
Build and test / build-and-test (push) Successful in 21m10s
Click link and see that system is working. Log in using the principal `admin@example.com`. Change something in the deployed system. Re-run the action. See that the database has reset.

**Major change** Added OAuth 2 login so no longer need modified web.xml with filter. Run `docker compose up` to start the local OAuth 2 authorization server to log in. Use the custom ticket form and enter the username you want to log in as in the "Principal" field.

Squashed all migrations since there are faulty ones that can't be applied to an empty database.

Reviewed-on: #15
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>
2024-12-19 10:44:48 +01:00
25117c8187 Switch authentication to OAuth 2 (#27)
All checks were successful
Build and test / build-and-test (push) Successful in 16m2s
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: #27
Reviewed-by: Nico Athanassiadis <nico@dsv.su.se>
2024-12-16 16:55:49 +01:00
1554d4bc27 Enforce code formatting via Prettier (#44)
All checks were successful
Build and test / build-and-test (push) Successful in 11m55s
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>
2024-12-02 14:17:59 +01:00
a2330ce2d5 Squash and fix migrations so they run against an empty schema (#24)
All checks were successful
Build and test / build-and-test (push) Successful in 6m51s
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 [ff4c5b58b40db5fcb7754c259c3854194668c1e1](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>
2024-12-02 10:31:20 +01:00
ff4c5b58b4 Allow changes to the reflection to be made after it's been submitted (#13)
All checks were successful
Build and test / build-and-test (push) Successful in 7m5s
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>
2024-11-21 19:20:47 +01:00
a9b8542576 Fix users getting stuck at a blank white page after logging in. (#16)
All checks were successful
Build and test / build-and-test (push) Successful in 6m41s
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>
2024-11-13 08:01:17 +01:00
ccac2c1cf8 Enable creating an API using Spring Web (#5)
All checks were successful
Build and test / build-and-test (push) Successful in 6m54s
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>
2024-11-06 11:23:28 +01:00