Public Access
feat(security): accept the auth cookie and guard against CSRF
Core reads the login token from the tbp_access cookie or a Bearer header, whichever verifies, and parses it once per request. Unsafe requests need an X-TBP-Client or Authorization header, CORS allows only the exact frontend origin, and a missing or expired token answers 401 instead of 403.
This commit is contained in:
1 parent
762b7898d3
commit
5d608b72e6
12 files changed
+362
-121
No files matched your search
@@ -0,0 +1,100 @@
|
||||
package core_service.configuration.security;
|
||||
|
||||
import core_service.shared.security.UserPrincipal;
|
||||
import core_service.shared.test.AbstractUnitTest;
|
||||
import jakarta.servlet.http.Cookie;
|
||||
import org.junit.jupiter.api.AfterEach;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.mockito.Mock;
|
||||
import org.springframework.mock.web.MockFilterChain;
|
||||
import org.springframework.mock.web.MockHttpServletRequest;
|
||||
import org.springframework.mock.web.MockHttpServletResponse;
|
||||
import org.springframework.security.core.Authentication;
|
||||
import org.springframework.security.core.context.SecurityContextHolder;
|
||||
|
||||
import java.util.Optional;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||
import static org.junit.jupiter.api.Assertions.assertNull;
|
||||
import static org.mockito.Mockito.lenient;
|
||||
|
||||
/**
|
||||
* Both login filters take the token from the access cookie or the Bearer header, the first one that verifies.
|
||||
*/
|
||||
class AccessTokenFiltersTest extends AbstractUnitTest {
|
||||
|
||||
private static final UserPrincipal USER = new UserPrincipal("42", "a@b.de", 76561198000000042L);
|
||||
|
||||
@Mock
|
||||
private JwtService jwtService;
|
||||
|
||||
@AfterEach
|
||||
void clearContext() {
|
||||
SecurityContextHolder.clearContext();
|
||||
}
|
||||
|
||||
private Object principalAfter(jakarta.servlet.Filter filter, MockHttpServletRequest request) throws Exception {
|
||||
filter.doFilter(request, new MockHttpServletResponse(), new MockFilterChain());
|
||||
Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
|
||||
return authentication == null ? null : authentication.getPrincipal();
|
||||
}
|
||||
|
||||
private void validToken(String token) {
|
||||
lenient().when(jwtService.parseLoginToken(token)).thenReturn(Optional.of(USER));
|
||||
}
|
||||
|
||||
@Test
|
||||
void cookieAuthenticates() throws Exception {
|
||||
validToken("good");
|
||||
MockHttpServletRequest request = new MockHttpServletRequest();
|
||||
request.setCookies(new Cookie(AccessTokens.ACCESS_COOKIE, "good"));
|
||||
|
||||
assertEquals(USER, principalAfter(new JwtAuthenticationFilter(jwtService), request));
|
||||
}
|
||||
|
||||
@Test
|
||||
void bearerAuthenticates() throws Exception {
|
||||
validToken("good");
|
||||
MockHttpServletRequest request = new MockHttpServletRequest();
|
||||
request.addHeader("Authorization", "Bearer good");
|
||||
|
||||
assertEquals(USER, principalAfter(new JwtAuthenticationFilter(jwtService), request));
|
||||
}
|
||||
|
||||
@Test
|
||||
void staleCookieFallsBackToTheBearerHeader() throws Exception {
|
||||
validToken("good");
|
||||
lenient().when(jwtService.parseLoginToken("stale")).thenReturn(Optional.empty());
|
||||
MockHttpServletRequest request = new MockHttpServletRequest();
|
||||
request.setCookies(new Cookie(AccessTokens.ACCESS_COOKIE, "stale"));
|
||||
request.addHeader("Authorization", "Bearer good");
|
||||
|
||||
assertEquals(USER, principalAfter(new JwtAuthenticationFilter(jwtService), request));
|
||||
}
|
||||
|
||||
@Test
|
||||
void invalidTokenStaysAnonymous() throws Exception {
|
||||
lenient().when(jwtService.parseLoginToken("bad")).thenReturn(Optional.empty());
|
||||
MockHttpServletRequest request = new MockHttpServletRequest();
|
||||
request.setCookies(new Cookie(AccessTokens.ACCESS_COOKIE, "bad"));
|
||||
|
||||
assertNull(principalAfter(new JwtAuthenticationFilter(jwtService), request));
|
||||
}
|
||||
|
||||
@Test
|
||||
void devFilter_UsesTheCookieUser() throws Exception {
|
||||
validToken("good");
|
||||
MockHttpServletRequest request = new MockHttpServletRequest();
|
||||
request.setCookies(new Cookie(AccessTokens.ACCESS_COOKIE, "good"));
|
||||
|
||||
assertEquals(USER, principalAfter(new DevJwtAuthenticationFilter(jwtService), request));
|
||||
}
|
||||
|
||||
@Test
|
||||
void devFilter_WithoutTokenUsesTheDummyUser() throws Exception {
|
||||
UserPrincipal principal = (UserPrincipal) principalAfter(new DevJwtAuthenticationFilter(jwtService),
|
||||
new MockHttpServletRequest());
|
||||
|
||||
assertEquals("1", principal.id());
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,56 @@
|
||||
package core_service.configuration.security;
|
||||
|
||||
import core_service.shared.test.AbstractIntegrationTest;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
import org.springframework.boot.webmvc.test.autoconfigure.AutoConfigureMockMvc;
|
||||
import org.springframework.test.web.servlet.MockMvc;
|
||||
|
||||
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.options;
|
||||
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
|
||||
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header;
|
||||
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
|
||||
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
|
||||
|
||||
/**
|
||||
* The dev profile logs every request in, so only the CSRF header and CORS decide here.
|
||||
*/
|
||||
@AutoConfigureMockMvc
|
||||
class ClientHeaderFilterTest extends AbstractIntegrationTest {
|
||||
|
||||
private static final String FRONTEND = "https://tbp.andreas-dahm.eu";
|
||||
|
||||
@Autowired
|
||||
private MockMvc mockMvc;
|
||||
|
||||
@Test
|
||||
void postWithoutClientHeader_Is403() throws Exception {
|
||||
mockMvc.perform(post("/api/library/sync"))
|
||||
.andExpect(status().isForbidden())
|
||||
.andExpect(jsonPath("$.code").value("CLIENT_HEADER_MISSING"));
|
||||
}
|
||||
|
||||
@Test
|
||||
void postWithClientHeader_PassesTheFilter() throws Exception {
|
||||
int status = mockMvc.perform(post("/api/library/sync").header(ClientHeaderFilter.CLIENT_HEADER, "test"))
|
||||
.andReturn().getResponse().getStatus();
|
||||
|
||||
org.junit.jupiter.api.Assertions.assertNotEquals(403, status);
|
||||
}
|
||||
|
||||
@Test
|
||||
void corsPreflight_AllowsOnlyTheFrontendWithCookies() throws Exception {
|
||||
mockMvc.perform(options("/api/library/sync")
|
||||
.header("Origin", FRONTEND)
|
||||
.header("Access-Control-Request-Method", "POST")
|
||||
.header("Access-Control-Request-Headers", "x-tbp-client"))
|
||||
.andExpect(status().isOk())
|
||||
.andExpect(header().string("Access-Control-Allow-Origin", FRONTEND))
|
||||
.andExpect(header().string("Access-Control-Allow-Credentials", "true"));
|
||||
|
||||
mockMvc.perform(options("/api/library/sync")
|
||||
.header("Origin", "https://evil.andreas-dahm.eu")
|
||||
.header("Access-Control-Request-Method", "POST"))
|
||||
.andExpect(status().isForbidden());
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,42 @@
|
||||
package core_service.configuration.security;
|
||||
|
||||
import core_service.shared.test.AbstractIntegrationTest;
|
||||
import jakarta.servlet.http.Cookie;
|
||||
import io.jsonwebtoken.Jwts;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
import org.springframework.boot.webmvc.test.autoconfigure.AutoConfigureMockMvc;
|
||||
import org.springframework.test.context.ActiveProfiles;
|
||||
import org.springframework.test.web.servlet.MockMvc;
|
||||
|
||||
import java.util.Date;
|
||||
|
||||
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
|
||||
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
|
||||
|
||||
/**
|
||||
* The real (non-dev) filter chain, where the token decides.
|
||||
*/
|
||||
@AutoConfigureMockMvc
|
||||
@ActiveProfiles(value = "prod", inheritProfiles = false)
|
||||
class ExpiredAccessTokenTest extends AbstractIntegrationTest {
|
||||
|
||||
@Autowired
|
||||
private MockMvc mockMvc;
|
||||
|
||||
private static String expiredAccessToken() {
|
||||
return Jwts.builder()
|
||||
.subject("a@b.de")
|
||||
.claim("userId", "42")
|
||||
.issuedAt(new Date(System.currentTimeMillis() - 120_000))
|
||||
.expiration(new Date(System.currentTimeMillis() - 60_000))
|
||||
.signWith(TEST_KEYS.getPrivate(), Jwts.SIG.EdDSA)
|
||||
.compact();
|
||||
}
|
||||
|
||||
@Test
|
||||
void expiredAccessCookie_Is401SoTheFrontendRefreshes() throws Exception {
|
||||
mockMvc.perform(get("/api/library/summary").cookie(new Cookie(AccessTokens.ACCESS_COOKIE, expiredAccessToken())))
|
||||
.andExpect(status().isUnauthorized());
|
||||
}
|
||||
}
|
||||
@@ -1,5 +1,6 @@
|
||||
package core_service.configuration.security;
|
||||
|
||||
import core_service.shared.security.UserPrincipal;
|
||||
import core_service.shared.test.AbstractUnitTest;
|
||||
import io.jsonwebtoken.Jwts;
|
||||
import org.junit.jupiter.api.BeforeEach;
|
||||
@@ -54,51 +55,40 @@ class JwtServiceTest extends AbstractUnitTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
void extractEmail_Success() {
|
||||
var email = "test@example.com";
|
||||
var token = signToken("0815", email, null);
|
||||
void parseLoginToken_ReturnsTheUser() {
|
||||
String token = signToken("0815", "test@example.com", 123456789L);
|
||||
|
||||
assertEquals(email, jwtService.extractEmail(token));
|
||||
assertEquals(Optional.of(new UserPrincipal("0815", "test@example.com", 123456789L)),
|
||||
jwtService.parseLoginToken(token));
|
||||
}
|
||||
|
||||
@Test
|
||||
void extractUserId_Success() {
|
||||
var userId = "0815";
|
||||
var token = signToken(userId, "test@example.com", null);
|
||||
|
||||
assertEquals(userId, jwtService.extractUserId(token));
|
||||
void parseLoginToken_RejectsGarbage() {
|
||||
assertEquals(Optional.empty(), jwtService.parseLoginToken("not-a-jwt"));
|
||||
}
|
||||
|
||||
@Test
|
||||
void extractSteamId_Success() {
|
||||
var steamId = 123456789L;
|
||||
var token = signToken("0815", "test@example.com", steamId);
|
||||
|
||||
assertEquals(steamId, jwtService.extractSteamId(token));
|
||||
}
|
||||
|
||||
@Test
|
||||
void isTokenValid_Success() {
|
||||
var token = signToken("0815", "test@example.com", null);
|
||||
|
||||
assertTrue(jwtService.isTokenValid(token));
|
||||
}
|
||||
|
||||
@Test
|
||||
void isTokenValid_Failure_garbage() {
|
||||
assertFalse(jwtService.isTokenValid("not-a-jwt"));
|
||||
}
|
||||
|
||||
@Test
|
||||
void isTokenValid_Failure_wrongKey() throws Exception {
|
||||
void parseLoginToken_RejectsAForeignKey() throws Exception {
|
||||
KeyPair other = KeyPairGenerator.getInstance("Ed25519").generateKeyPair();
|
||||
String token = Jwts.builder()
|
||||
.subject("test@example.com")
|
||||
.claim("userId", "0815")
|
||||
.expiration(new Date(System.currentTimeMillis() + 60_000))
|
||||
.signWith(other.getPrivate(), Jwts.SIG.EdDSA)
|
||||
.compact();
|
||||
|
||||
assertFalse(jwtService.isTokenValid(token));
|
||||
assertEquals(Optional.empty(), jwtService.parseLoginToken(token));
|
||||
}
|
||||
|
||||
@Test
|
||||
void parseLoginToken_RejectsATokenWithoutUserId() {
|
||||
String token = Jwts.builder()
|
||||
.subject("test@example.com")
|
||||
.expiration(new Date(System.currentTimeMillis() + 60_000))
|
||||
.signWith(signingKey, Jwts.SIG.EdDSA)
|
||||
.compact();
|
||||
|
||||
assertEquals(Optional.empty(), jwtService.parseLoginToken(token));
|
||||
}
|
||||
|
||||
private String signTicket(String type, String profileUserId, long expiresInMs) {
|
||||
@@ -130,7 +120,7 @@ class JwtServiceTest extends AbstractUnitTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
void isTokenValid_ProfileTicketIsNoLogin() {
|
||||
assertFalse(jwtService.isTokenValid(signTicket("public-profile", "42", 60_000)));
|
||||
void parseLoginToken_ProfileTicketIsNoLogin() {
|
||||
assertEquals(Optional.empty(), jwtService.parseLoginToken(signTicket("public-profile", "42", 60_000)));
|
||||
}
|
||||
}
|
||||
@@ -7,6 +7,7 @@ import core_service.modules.library.domain.UserGame;
|
||||
import core_service.modules.library.domain.UserGameId;
|
||||
import core_service.modules.library.repo.SteamGameRepository;
|
||||
import core_service.modules.library.repo.UserGameRepository;
|
||||
import core_service.configuration.security.ClientHeaderFilter;
|
||||
import core_service.shared.test.AbstractIntegrationTest;
|
||||
import org.junit.jupiter.api.BeforeEach;
|
||||
import org.junit.jupiter.api.Test;
|
||||
@@ -56,7 +57,7 @@ class LibraryControllerBacklogTest extends AbstractIntegrationTest {
|
||||
|
||||
@Test
|
||||
void updateBacklog_StoresAllFields() throws Exception {
|
||||
mockMvc.perform(put("/api/library/games/220/backlog").contentType(MediaType.APPLICATION_JSON)
|
||||
mockMvc.perform(put("/api/library/games/220/backlog").header(ClientHeaderFilter.CLIENT_HEADER, "test").contentType(MediaType.APPLICATION_JSON)
|
||||
.content("{\"status\":\"FINISHED\",\"rating\":4,\"notes\":\"Great ending\",\"priority\":\"HIGH\"}"))
|
||||
.andExpect(status().isOk())
|
||||
.andExpect(jsonPath("$.status").value("FINISHED"))
|
||||
@@ -76,7 +77,7 @@ class LibraryControllerBacklogTest extends AbstractIntegrationTest {
|
||||
game.setNotes("old");
|
||||
userGameRepository.save(game);
|
||||
|
||||
mockMvc.perform(put("/api/library/games/220/backlog").contentType(MediaType.APPLICATION_JSON)
|
||||
mockMvc.perform(put("/api/library/games/220/backlog").header(ClientHeaderFilter.CLIENT_HEADER, "test").contentType(MediaType.APPLICATION_JSON)
|
||||
.content("{\"status\":\"DROPPED\",\"notes\":\" \"}"))
|
||||
.andExpect(status().isOk());
|
||||
|
||||
@@ -95,7 +96,7 @@ class LibraryControllerBacklogTest extends AbstractIntegrationTest {
|
||||
"{\"status\":\"PLAYING\",\"rating\":6}",
|
||||
"{\"status\":\"PLAYING\",\"priority\":\"URGENT\"}"})
|
||||
void updateBacklog_InvalidBody_Is400AndStoresNothing(String body) throws Exception {
|
||||
mockMvc.perform(put("/api/library/games/220/backlog").contentType(MediaType.APPLICATION_JSON).content(body))
|
||||
mockMvc.perform(put("/api/library/games/220/backlog").header(ClientHeaderFilter.CLIENT_HEADER, "test").contentType(MediaType.APPLICATION_JSON).content(body))
|
||||
.andExpect(status().isBadRequest());
|
||||
|
||||
assertEquals(GameStatus.BACKLOG, stored().getStatus());
|
||||
@@ -103,7 +104,7 @@ class LibraryControllerBacklogTest extends AbstractIntegrationTest {
|
||||
|
||||
@Test
|
||||
void updateBacklog_FractionalRating_Is400() throws Exception {
|
||||
mockMvc.perform(put("/api/library/games/220/backlog").contentType(MediaType.APPLICATION_JSON)
|
||||
mockMvc.perform(put("/api/library/games/220/backlog").header(ClientHeaderFilter.CLIENT_HEADER, "test").contentType(MediaType.APPLICATION_JSON)
|
||||
.content("{\"status\":\"PLAYING\",\"rating\":4.7}"))
|
||||
.andExpect(status().isBadRequest());
|
||||
|
||||
@@ -113,7 +114,7 @@ class LibraryControllerBacklogTest extends AbstractIntegrationTest {
|
||||
@Test
|
||||
void updateBacklog_NotesTooLong_Is400() throws Exception {
|
||||
String notes = "x".repeat(2001);
|
||||
mockMvc.perform(put("/api/library/games/220/backlog").contentType(MediaType.APPLICATION_JSON)
|
||||
mockMvc.perform(put("/api/library/games/220/backlog").header(ClientHeaderFilter.CLIENT_HEADER, "test").contentType(MediaType.APPLICATION_JSON)
|
||||
.content("{\"status\":\"PLAYING\",\"notes\":\"" + notes + "\"}"))
|
||||
.andExpect(status().isBadRequest());
|
||||
}
|
||||
@@ -124,7 +125,7 @@ class LibraryControllerBacklogTest extends AbstractIntegrationTest {
|
||||
game.setRemovedAt(Instant.now());
|
||||
userGameRepository.save(game);
|
||||
|
||||
mockMvc.perform(put("/api/library/games/220/backlog").contentType(MediaType.APPLICATION_JSON)
|
||||
mockMvc.perform(put("/api/library/games/220/backlog").header(ClientHeaderFilter.CLIENT_HEADER, "test").contentType(MediaType.APPLICATION_JSON)
|
||||
.content("{\"status\":\"PLAYING\"}"))
|
||||
.andExpect(status().isNotFound())
|
||||
.andExpect(jsonPath("$.code").value("GAME_NOT_FOUND"));
|
||||
@@ -132,7 +133,7 @@ class LibraryControllerBacklogTest extends AbstractIntegrationTest {
|
||||
|
||||
@Test
|
||||
void updateBacklog_GameNotOwned_Is404() throws Exception {
|
||||
mockMvc.perform(put("/api/library/games/999/backlog").contentType(MediaType.APPLICATION_JSON)
|
||||
mockMvc.perform(put("/api/library/games/999/backlog").header(ClientHeaderFilter.CLIENT_HEADER, "test").contentType(MediaType.APPLICATION_JSON)
|
||||
.content("{\"status\":\"PLAYING\"}"))
|
||||
.andExpect(status().isNotFound());
|
||||
}
|
||||
@@ -145,7 +146,7 @@ class LibraryControllerBacklogTest extends AbstractIntegrationTest {
|
||||
game.setPriority(GamePriority.HIGH);
|
||||
userGameRepository.save(game);
|
||||
|
||||
mockMvc.perform(put("/api/library/games/220/status").contentType(MediaType.APPLICATION_JSON)
|
||||
mockMvc.perform(put("/api/library/games/220/status").header(ClientHeaderFilter.CLIENT_HEADER, "test").contentType(MediaType.APPLICATION_JSON)
|
||||
.content("{\"status\":\"PLAYING\"}"))
|
||||
.andExpect(status().isOk())
|
||||
.andExpect(jsonPath("$.status").value("PLAYING"))
|
||||
@@ -160,7 +161,7 @@ class LibraryControllerBacklogTest extends AbstractIntegrationTest {
|
||||
|
||||
@Test
|
||||
void updateStatus_MissingStatus_Is400() throws Exception {
|
||||
mockMvc.perform(put("/api/library/games/220/status").contentType(MediaType.APPLICATION_JSON)
|
||||
mockMvc.perform(put("/api/library/games/220/status").header(ClientHeaderFilter.CLIENT_HEADER, "test").contentType(MediaType.APPLICATION_JSON)
|
||||
.content("{}"))
|
||||
.andExpect(status().isBadRequest());
|
||||
|
||||
@@ -173,7 +174,7 @@ class LibraryControllerBacklogTest extends AbstractIntegrationTest {
|
||||
game.setRemovedAt(Instant.now());
|
||||
userGameRepository.save(game);
|
||||
|
||||
mockMvc.perform(put("/api/library/games/220/status").contentType(MediaType.APPLICATION_JSON)
|
||||
mockMvc.perform(put("/api/library/games/220/status").header(ClientHeaderFilter.CLIENT_HEADER, "test").contentType(MediaType.APPLICATION_JSON)
|
||||
.content("{\"status\":\"PLAYING\"}"))
|
||||
.andExpect(status().isNotFound())
|
||||
.andExpect(jsonPath("$.code").value("GAME_NOT_FOUND"));
|
||||
@@ -181,7 +182,7 @@ class LibraryControllerBacklogTest extends AbstractIntegrationTest {
|
||||
|
||||
@Test
|
||||
void updateStatus_GameNotOwned_Is404() throws Exception {
|
||||
mockMvc.perform(put("/api/library/games/999/status").contentType(MediaType.APPLICATION_JSON)
|
||||
mockMvc.perform(put("/api/library/games/999/status").header(ClientHeaderFilter.CLIENT_HEADER, "test").contentType(MediaType.APPLICATION_JSON)
|
||||
.content("{\"status\":\"PLAYING\"}"))
|
||||
.andExpect(status().isNotFound());
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user