Skip to content

Commit

Permalink
Turn on CSRF protection. Refactor the integration test setup to handl…
Browse files Browse the repository at this point in the history 
…e CSRF tokens. Update tests for CSRF tokens.
  • Loading branch information
@markpatton
markpatton committed May 30, 2024
1 parent bede17c commit aa073fd
Show file tree
Hide file tree
Showing 16 changed files with 348 additions and 96 deletions.
22 changes: 22 additions & 0 deletions pass-core-main/src/main/java/org/eclipse/pass/main/security/CsrfCookieFilter.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package org.eclipse.pass.main.security;

import java.io.IOException;

import jakarta.servlet.FilterChain;
import jakarta.servlet.ServletException;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import org.springframework.security.web.csrf.CsrfToken;
import org.springframework.web.filter.OncePerRequestFilter;

public class CsrfCookieFilter extends OncePerRequestFilter {
@Override
protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain)
throws ServletException, IOException {
CsrfToken csrfToken = (CsrfToken) request.getAttribute("_csrf");
// Render the token value to a cookie by causing the deferred token to be loaded
csrfToken.getToken();

filterChain.doFilter(request, response);
}
}
18 changes: 15 additions & 3 deletions pass-core-main/src/main/java/org/eclipse/pass/main/security/SecurityConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
import org.springframework.security.config.annotation.web.configurers.AnonymousConfigurer;
import org.springframework.security.config.annotation.web.configurers.CsrfConfigurer;
import org.springframework.security.config.annotation.web.configurers.FormLoginConfigurer;
import org.springframework.security.saml2.provider.service.registration.RelyingPartyRegistrationRepository;
import org.springframework.security.saml2.provider.service.web.DefaultRelyingPartyRegistrationResolver;
Expand All @@ -37,6 +36,7 @@
import org.springframework.security.saml2.provider.service.web.authentication.Saml2WebSsoAuthenticationFilter;
import org.springframework.security.web.SecurityFilterChain;
import org.springframework.security.web.authentication.logout.CookieClearingLogoutHandler;
import org.springframework.security.web.csrf.CookieCsrfTokenRepository;
import org.springframework.security.web.header.writers.ContentSecurityPolicyHeaderWriter;
import org.springframework.security.web.header.writers.DelegatingRequestMatcherHeaderWriter;
import org.springframework.security.web.savedrequest.HttpSessionRequestCache;
Expand Down Expand Up @@ -77,10 +77,16 @@ public class SecurityConfiguration {
@Bean
public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
// Disable unused functionality
http.csrf(CsrfConfigurer::disable);
http.formLogin(FormLoginConfigurer::disable);
http.anonymous(AnonymousConfigurer::disable);

// Enable CSRF protection using a cookie to send the token
// Make sure the cookie value can be parsed when returned in a header
// Do not protect /logout so it can be triggered with GET
http.csrf(csrf -> csrf.csrfTokenRepository(CookieCsrfTokenRepository.withHttpOnlyFalse())
.ignoringRequestMatchers("/logout")
.csrfTokenRequestHandler(new SpaCsrfTokenRequestHandler()));

// Set Content Security Policy header only for /app/
ContentSecurityPolicyHeaderWriter cspHeaderWriter = new ContentSecurityPolicyHeaderWriter();
cspHeaderWriter.setPolicyDirectives(contentSecurityPolicy);
Expand Down Expand Up @@ -111,6 +117,7 @@ public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
loginProcessingUrl(loginProcessingPath));

http.saml2Metadata(Customizer.withDefaults());

http.saml2Logout(Customizer.withDefaults());

// Delete specified cookies on logout.
Expand All @@ -124,12 +131,17 @@ public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
return c;
}).toArray(Cookie[]::new);

// Allow GET on /logout
CookieClearingLogoutHandler logoutHandler = new CookieClearingLogoutHandler(cookies);
http.logout(l -> l.logoutSuccessUrl(logoutSuccessUrl).addLogoutHandler(logoutHandler));
http.logout(l -> l.logoutSuccessUrl(logoutSuccessUrl).
logoutRequestMatcher(new AntPathRequestMatcher("/logout")).addLogoutHandler(logoutHandler));

// Map SAML user to PASS user
http.addFilterAfter(passAuthFilter, Saml2WebSsoAuthenticationFilter.class);

// Ensure that CSRF cookie is set
http.addFilterAfter(new CsrfCookieFilter(), Saml2WebSsoAuthenticationFilter.class);

return http.build();
}

Expand Down
46 changes: 46 additions & 0 deletions pass-core-main/src/main/java/org/eclipse/pass/main/security/SpaCsrfTokenRequestHandler.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package org.eclipse.pass.main.security;

import java.util.function.Supplier;

import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import org.springframework.security.web.csrf.CsrfToken;
import org.springframework.security.web.csrf.CsrfTokenRequestAttributeHandler;
import org.springframework.security.web.csrf.CsrfTokenRequestHandler;
import org.springframework.security.web.csrf.XorCsrfTokenRequestAttributeHandler;

public class SpaCsrfTokenRequestHandler extends CsrfTokenRequestAttributeHandler {
private final CsrfTokenRequestHandler delegate = new XorCsrfTokenRequestAttributeHandler();

@Override
public void handle(HttpServletRequest request, HttpServletResponse response, Supplier<CsrfToken> csrfToken) {
/*
* Always use XorCsrfTokenRequestAttributeHandler to provide BREACH protection of
* the CsrfToken when it is rendered in the response body.
*/
this.delegate.handle(request, response, csrfToken);
}

@Override
public String resolveCsrfTokenValue(HttpServletRequest request, CsrfToken csrfToken) {
/*
* If the request contains a request header, use CsrfTokenRequestAttributeHandler
* to resolve the CsrfToken. This applies when a single-page application includes
* the header value automatically, which was obtained via a cookie containing the
* raw CsrfToken.
*/
String token = request.getHeader(csrfToken.getHeaderName());

if (token != null && !token.isEmpty()) {
return super.resolveCsrfTokenValue(request, csrfToken);
}

/*
* In all other cases (e.g. if the request contains a request parameter), use
* XorCsrfTokenRequestAttributeHandler to resolve the CsrfToken. This applies
* when a server-side rendered form includes the _csrf request parameter as a
* hidden input.
*/
return this.delegate.resolveCsrfTokenValue(request, csrfToken);
}
}
34 changes: 20 additions & 14 deletions pass-core-main/src/test/java/org/eclipse/pass/doi/service/DoiServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import okhttp3.OkHttpClient;
import okhttp3.Request;
import okhttp3.Response;
import org.eclipse.pass.main.IntegrationTest;
import org.eclipse.pass.main.SimpleIntegrationTest;
import org.eclipse.pass.object.PassClient;
import org.eclipse.pass.object.PassClientResult;
import org.eclipse.pass.object.PassClientSelector;
Expand All @@ -25,7 +25,7 @@
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;

public class DoiServiceTest extends IntegrationTest {
public class DoiServiceTest extends SimpleIntegrationTest {

@Autowired
protected RefreshableElide refreshableElide;
Expand All @@ -52,9 +52,10 @@ public void invalidDoiTest() throws Exception {
Call call = httpClient.newCall(okHttpRequest);
try (Response okHttpResponse = call.execute()) {
assertEquals(400, okHttpResponse.code());
assert okHttpResponse.body() != null;
String body = okHttpResponse.body().string();
assert body != null;
assertEquals("{\"error\":\"Supplied DOI is not in valid DOI format.\"}",
okHttpResponse.body().string());
body);

}
}
Expand All @@ -74,9 +75,10 @@ public void nullDoiTest() throws Exception {
Call call = httpClient.newCall(okHttpRequest);
try (Response okHttpResponse = call.execute()) {
assertEquals(400, okHttpResponse.code());
assert okHttpResponse.body() != null;
String body = okHttpResponse.body().string();
assert body != null;
assertEquals("{\"error\":\"Supplied DOI is not in valid DOI format.\"}",
okHttpResponse.body().string());
body);

}
}
Expand All @@ -97,9 +99,10 @@ public void noSuchDoiTest() throws Exception {
Call call = httpClient.newCall(okHttpRequest);
try (Response okHttpResponse = call.execute()) {
assertEquals(404, okHttpResponse.code());
assert okHttpResponse.body() != null;
String body = okHttpResponse.body().string();
assert body != null;
assertEquals("{\"error\":\"The resource for DOI 10.1212/abc.DEF could not be found on Crossref.\"}",
okHttpResponse.body().string());
body);

}
}
Expand All @@ -123,9 +126,10 @@ public void bookDoiFailTest() throws Exception {
Call call = httpClient.newCall(okHttpRequest);
try (Response okHttpResponse = call.execute()) {
assertEquals(422, okHttpResponse.code());
assert okHttpResponse.body() != null;
String body = okHttpResponse.body().string();
assert body != null;
assertEquals("{\"error\":\"Insufficient information to locate or specify a journal entry.\"}",
okHttpResponse.body().string());
body);
}
}

Expand Down Expand Up @@ -161,8 +165,9 @@ public void realJournalTest() {
Call call = httpClient.newCall(okHttpRequest);
try (Response okHttpResponse = call.execute()) {
assertEquals(200, okHttpResponse.code());
assert okHttpResponse.body() != null;
JsonReader jsonReader1 = Json.createReader(new StringReader(okHttpResponse.body().string()));
String body = okHttpResponse.body().string();
assert body != null;
JsonReader jsonReader1 = Json.createReader(new StringReader(body));
JsonObject successReport = jsonReader1.readObject();
jsonReader1.close();
assertNotNull(successReport.getString("journal-id"));
Expand All @@ -180,8 +185,9 @@ public void realJournalTest() {
//verify that the returned object for the same request has the right id
call = httpClient.newCall(okHttpRequest);
try (Response okHttpResponse = call.execute()) {
assert okHttpResponse.body() != null;
JsonReader jsonReader2 = Json.createReader(new StringReader(okHttpResponse.body().string()));
String body = okHttpResponse.body().string();
assert body != null;
JsonReader jsonReader2 = Json.createReader(new StringReader(body));
JsonObject successReport = jsonReader2.readObject();
jsonReader2.close();
assertEquals(id, successReport.getString("journal-id"));
Expand Down
50 changes: 34 additions & 16 deletions ...core-main/src/test/java/org/eclipse/pass/file/service/storage/FileStorageServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,18 +37,18 @@
import java.util.Objects;

import io.ocfl.api.exception.NotFoundException;
import okhttp3.Credentials;
import okhttp3.MediaType;
import okhttp3.MultipartBody;
import okhttp3.OkHttpClient;
import okhttp3.Request;
import okhttp3.RequestBody;
import okhttp3.Response;
import org.eclipse.pass.file.service.PassFileServiceController;
import org.eclipse.pass.main.IntegrationTest;
import org.eclipse.pass.main.SimpleIntegrationTest;
import org.json.JSONException;
import org.json.JSONObject;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.junit.jupiter.MockitoExtension;
Expand All @@ -68,20 +68,30 @@
* @see FileStorageService
*/
@ExtendWith(MockitoExtension.class)
public class FileStorageServiceTest extends IntegrationTest {
public class FileStorageServiceTest extends SimpleIntegrationTest {
protected final String USER_NAME = "USER1";
protected final String USER_NAME2 = "USER2";
private final String credentialsBackend = Credentials.basic(BACKEND_USER, BACKEND_PASSWORD);
private final OkHttpClient httpClient = new OkHttpClient();

public static final MediaType MEDIA_TYPE_TEXT
= MediaType.parse("text/plain");
public static final MediaType MEDIA_TYPE_APPLICATION
= MediaType.parse("application/octet-stream");

private OkHttpClient httpClient;

@Autowired protected PassFileServiceController passFileServiceController;
@Autowired protected FileStorageService storageService;
@Autowired protected StorageProperties storageProperties;

@BeforeEach
protected void setupClient() throws IOException {
httpClient = newOkhttpClient();
}

public String getCsrfToken() throws IOException {
return getCsrfToken(httpClient);
}

/**
* Cleanup the FileStorageService after testing. Deletes the root directory.
*/
Expand Down Expand Up @@ -253,7 +263,7 @@ void getFileByIdUsingController() throws IOException {
.build();
Request request = new Request.Builder()
.url(url)
.header("Authorization", credentialsBackend)
.header("Authorization", BACKEND_CREDENTIALS)
.get()
.build();

Expand Down Expand Up @@ -281,7 +291,8 @@ void uploadFile() throws IOException {
Request request = new Request.Builder()
.url(url)
.post(requestBody)
.addHeader("Authorization", credentialsBackend)
.addHeader("Authorization", BACKEND_CREDENTIALS)
.addHeader("X-XSRF-TOKEN", getCsrfToken())
.build();

try (Response response = httpClient.newCall(request).execute()) {
Expand Down Expand Up @@ -311,7 +322,8 @@ void uploadFileWithUnknownType() throws IOException, JSONException {
Request request = new Request.Builder()
.url(url)
.post(requestBody)
.addHeader("Authorization", credentialsBackend)
.addHeader("Authorization", BACKEND_CREDENTIALS)
.addHeader("X-XSRF-TOKEN", getCsrfToken())
.build();

try (Response response = httpClient.newCall(request).execute()) {
Expand All @@ -321,7 +333,7 @@ void uploadFileWithUnknownType() throws IOException, JSONException {

// Download
Request dlRequest = new Request.Builder().url(url + "/" + id).
addHeader("Authorization", credentialsBackend).build();
addHeader("Authorization", BACKEND_CREDENTIALS).build();

try (Response dlResponse = httpClient.newCall(dlRequest).execute()) {
assertEquals(HttpStatus.OK.value(), dlResponse.code());
Expand Down Expand Up @@ -352,7 +364,8 @@ void uploadLargeFile() throws IOException, JSONException {
Request request = new Request.Builder()
.url(url)
.post(requestBody)
.addHeader("Authorization", credentialsBackend)
.addHeader("Authorization", BACKEND_CREDENTIALS)
.addHeader("X-XSRF-TOKEN", getCsrfToken())
.build();

try (Response response = httpClient.newCall(request).execute()) {
Expand All @@ -362,7 +375,7 @@ void uploadLargeFile() throws IOException, JSONException {

// Download
Request dlRequest = new Request.Builder().url(url + "/" + id).
addHeader("Authorization", credentialsBackend).build();
addHeader("Authorization", BACKEND_CREDENTIALS).build();

try (Response dlResponse = httpClient.newCall(dlRequest).execute()) {
assertEquals(HttpStatus.OK.value(), dlResponse.code());
Expand All @@ -389,7 +402,8 @@ void uploadFileMissingFile() throws IOException {
Request request = new Request.Builder()
.url(url)
.post(requestBody)
.addHeader("Authorization", credentialsBackend)
.addHeader("Authorization", BACKEND_CREDENTIALS)
.addHeader("X-XSRF-TOKEN", getCsrfToken())
.build();

try (Response response = httpClient.newCall(request).execute()) {
Expand All @@ -414,7 +428,8 @@ void uploadFileMissingFileName() throws IOException {
Request request = new Request.Builder()
.url(url)
.post(requestBody)
.addHeader("Authorization", credentialsBackend)
.addHeader("Authorization", BACKEND_CREDENTIALS)
.addHeader("X-XSRF-TOKEN", getCsrfToken())
.build();

try (Response response = httpClient.newCall(request).execute()) {
Expand All @@ -436,7 +451,8 @@ void deleteFileUsingFileServiceController() throws IOException {
Request request = new Request.Builder()
.url(url)
.delete()
.addHeader("Authorization", credentialsBackend)
.addHeader("Authorization", BACKEND_CREDENTIALS)
.addHeader("X-XSRF-TOKEN", getCsrfToken())
.build();

try (Response response = httpClient.newCall(request).execute()) {
Expand All @@ -461,7 +477,8 @@ void deleteFileUsingFileServiceControllerUserPermissionException() throws IOExce
Request request = new Request.Builder()
.url(url)
.delete()
.addHeader("Authorization", credentialsBackend)
.addHeader("Authorization", BACKEND_CREDENTIALS)
.addHeader("X-XSRF-TOKEN", getCsrfToken())
.build();

try (Response response = httpClient.newCall(request).execute()) {
Expand All @@ -485,7 +502,8 @@ void deleteFileUsingFileServiceControllerDeleteException() throws IOException {
Request request = new Request.Builder()
.url(url)
.delete()
.addHeader("Authorization", credentialsBackend)
.addHeader("Authorization", BACKEND_CREDENTIALS)
.addHeader("X-XSRF-TOKEN", getCsrfToken())
.build();

try (Response response = httpClient.newCall(request).execute()) {
Expand Down
Loading

0 comments on commit aa073fd

Please sign in to comment.