From f8e3b07b027f9f743a1fe2f976142e86f2ec2145 Mon Sep 17 00:00:00 2001 From: enimaloc Date: Sat, 14 Nov 2026 06:54:31 +0100 Subject: [PATCH 1/2] fix: link Steam, Xbox and Battle.net after catapult-api restarts A link's callback reads the user from catapult-api's in-memory session, lost on every restart while catapult-web still shows the user logged in. catapult-web now relays the user's JWT when a link starts, and catapult-api restores its session from it, replacing a stale session of another user. --- .../security/LinkSessionRestoreFilter.java | 111 +++++++++++++ .../LinkSessionRestoreFilterTest.java | 157 ++++++++++++++++++ .../catapult/relay/ApiRelayController.java | 19 +++ .../relay/ApiRelayControllerTest.java | 29 ++++ 4 files changed, 316 insertions(+) create mode 100644 catapult-api/src/main/java/fr/enimaloc/catapult/security/LinkSessionRestoreFilter.java create mode 100644 catapult-api/src/test/java/fr/enimaloc/catapult/security/LinkSessionRestoreFilterTest.java diff --git a/catapult-api/src/main/java/fr/enimaloc/catapult/security/LinkSessionRestoreFilter.java b/catapult-api/src/main/java/fr/enimaloc/catapult/security/LinkSessionRestoreFilter.java new file mode 100644 index 000000000..60960a2e5 --- /dev/null +++ b/catapult-api/src/main/java/fr/enimaloc/catapult/security/LinkSessionRestoreFilter.java @@ -0,0 +1,111 @@ +package fr.enimaloc.catapult.security; + +import fr.enimaloc.catapult.domain.account.UserAccount; +import fr.enimaloc.catapult.repository.account.UserAccountRepository; +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import jakarta.servlet.http.HttpSession; +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.springframework.core.Ordered; +import org.springframework.core.annotation.Order; +import org.springframework.http.HttpHeaders; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.context.SecurityContext; +import org.springframework.security.core.context.SecurityContextImpl; +import org.springframework.security.oauth2.jwt.Jwt; +import org.springframework.security.oauth2.jwt.JwtDecoder; +import org.springframework.security.oauth2.jwt.JwtException; +import org.springframework.security.web.context.HttpSessionSecurityContextRepository; +import org.springframework.stereotype.Component; +import org.springframework.web.filter.OncePerRequestFilter; + +import java.io.IOException; +import java.util.List; +import java.util.Optional; +import java.util.UUID; + +/** + * Restores the Twitch login of catapult-api's own session when a secondary link (Steam, Xbox, Battle.net) starts. + * + *

The link's callback finds the user in that session ({@link CatapultOAuth2UserService}), which lives in memory: + * a restart of catapult-api, or another replica, loses it while catapult-web still shows the user logged in. So on + * {@code /oauth2/authorization/}, catapult-web relays the user's JWT as a bearer token, checked by the same + * {@link JwtDecoder} as {@code /api/**}. The user it names replaces the session's, if any: a stale session may still + * hold the account logged in before, and the link must go to the one catapult-web shows. + * + *

Must run BEFORE Spring Security's chain, which loads the session's context and redirects to the provider. + */ +@Slf4j +@Component +@Order(Ordered.HIGHEST_PRECEDENCE + 1) +@RequiredArgsConstructor +public class LinkSessionRestoreFilter extends OncePerRequestFilter { + + static final String PATH_PREFIX = "/oauth2/authorization/"; + private static final String BEARER = "Bearer "; + + private final JwtDecoder jwtDecoder; + private final UserAccountRepository users; + + @Override + protected boolean shouldNotFilter(HttpServletRequest request) { + String uri = request.getRequestURI(); + // Twitch is the login itself: nothing to restore before it + return !uri.startsWith(PATH_PREFIX) || uri.equals(PATH_PREFIX + "twitch"); + } + + @Override + protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, + FilterChain chain) throws ServletException, IOException { + bearerUser(request) + .filter(user -> !user.getUserAccount().getId().equals(sessionUserId(request.getSession(false)))) + .ifPresent(user -> restore(request, user)); + chain.doFilter(request, response); + } + + private static UUID sessionUserId(HttpSession session) { + if (session != null + && session.getAttribute(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY) + instanceof SecurityContext context + && context.getAuthentication() != null + && context.getAuthentication().getPrincipal() instanceof CatapultOAuth2User user) { + return user.getUserAccount().getId(); + } + return null; + } + + private Optional bearerUser(HttpServletRequest request) { + String header = request.getHeader(HttpHeaders.AUTHORIZATION); + if (header == null || !header.startsWith(BEARER)) return Optional.empty(); + Jwt jwt; + UUID id; + try { + jwt = jwtDecoder.decode(header.substring(BEARER.length()).trim()); + id = UUID.fromString(jwt.getSubject()); + } catch (JwtException | IllegalArgumentException | NullPointerException e) { + log.info("Secondary link started with an unusable token ({})", e.getClass().getSimpleName()); + return Optional.empty(); + } + List roles = Optional.ofNullable(jwt.getClaimAsStringList("roles")).orElse(List.of()); + return users.findById(id) + .filter(account -> account.getStatus() == UserAccount.Status.ACTIVE) + .map(account -> new CatapultOAuth2User(null, account, roles.contains("ROLE_ADMIN"))); + } + + private static void restore(HttpServletRequest request, CatapultOAuth2User user) { + HttpSession session = request.getSession(false); + if (session == null) { + request.getSession(true); + } else { + // A session that gains or changes its user gets a new id (session fixation) + request.changeSessionId(); + } + SecurityContext context = new SecurityContextImpl( + UsernamePasswordAuthenticationToken.authenticated(user, null, user.getAuthorities())); + request.getSession().setAttribute(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY, context); + log.info("Restored the session of user {} for a secondary link", user.getUserAccount().getId()); + } +} diff --git a/catapult-api/src/test/java/fr/enimaloc/catapult/security/LinkSessionRestoreFilterTest.java b/catapult-api/src/test/java/fr/enimaloc/catapult/security/LinkSessionRestoreFilterTest.java new file mode 100644 index 000000000..9fcae09de --- /dev/null +++ b/catapult-api/src/test/java/fr/enimaloc/catapult/security/LinkSessionRestoreFilterTest.java @@ -0,0 +1,157 @@ +package fr.enimaloc.catapult.security; + +import fr.enimaloc.catapult.domain.account.UserAccount; +import fr.enimaloc.catapult.repository.account.UserAccountRepository; +import jakarta.servlet.FilterChain; +import org.junit.jupiter.api.Test; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.mock.web.MockHttpSession; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.GrantedAuthority; +import org.springframework.security.core.context.SecurityContext; +import org.springframework.security.core.context.SecurityContextImpl; +import org.springframework.security.oauth2.jwt.BadJwtException; +import org.springframework.security.oauth2.jwt.Jwt; +import org.springframework.security.oauth2.jwt.JwtDecoder; +import org.springframework.security.web.context.HttpSessionSecurityContextRepository; + +import java.time.Instant; +import java.util.List; +import java.util.Optional; +import java.util.UUID; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +/** LinkSessionRestoreFilter: a secondary link finds its user although catapult-api's session was lost. */ +class LinkSessionRestoreFilterTest { + + private final JwtDecoder decoder = mock(JwtDecoder.class); + private final UserAccountRepository users = mock(UserAccountRepository.class); + private final LinkSessionRestoreFilter filter = new LinkSessionRestoreFilter(decoder, users); + + private static UserAccount account(UUID id, UserAccount.Status status) { + UserAccount account = new UserAccount(); + account.setId(id); + account.setTwitchId("tw-" + id); + account.setStatus(status); + return account; + } + + private static Jwt jwt(UUID subject, List roles) { + return Jwt.withTokenValue("t").header("alg", "HS256").subject(subject.toString()).claim("roles", roles) + .issuedAt(Instant.now()).build(); + } + + private static MockHttpServletRequest linkStart(String provider, String token) { + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/oauth2/authorization/" + provider); + request.setRequestURI("/oauth2/authorization/" + provider); + if (token != null) request.addHeader("Authorization", "Bearer " + token); + return request; + } + + private static CatapultOAuth2User sessionUser(MockHttpServletRequest request) { + if (request.getSession(false) == null) return null; + Object context = request.getSession(false) + .getAttribute(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY); + return context instanceof SecurityContext c && c.getAuthentication() != null + && c.getAuthentication().getPrincipal() instanceof CatapultOAuth2User user ? user : null; + } + + @Test + void aLostSession_getsTheUserTheTokenNames() throws Exception { + UUID id = UUID.randomUUID(); + when(decoder.decode("good")).thenReturn(jwt(id, List.of("ROLE_USER", "ROLE_ADMIN"))); + when(users.findById(id)).thenReturn(Optional.of(account(id, UserAccount.Status.ACTIVE))); + MockHttpServletRequest request = linkStart("battlenet", "good"); + FilterChain chain = mock(FilterChain.class); + MockHttpServletResponse response = new MockHttpServletResponse(); + + filter.doFilter(request, response, chain); + + CatapultOAuth2User user = sessionUser(request); + assertThat(user).isNotNull(); + assertThat(user.getUserAccount().getId()).isEqualTo(id); + assertThat(user.getAuthorities()).extracting(GrantedAuthority::getAuthority).contains("ROLE_ADMIN"); + verify(chain).doFilter(request, response); + } + + @Test + void aStaleSessionOfAnotherUser_isReplaced_underANewSessionId() throws Exception { + UUID stale = UUID.randomUUID(); + UUID current = UUID.randomUUID(); + when(decoder.decode("good")).thenReturn(jwt(current, List.of("ROLE_USER"))); + when(users.findById(current)).thenReturn(Optional.of(account(current, UserAccount.Status.ACTIVE))); + MockHttpServletRequest request = linkStart("xbox", "good"); + MockHttpSession session = new MockHttpSession(); + CatapultOAuth2User old = new CatapultOAuth2User(null, account(stale, UserAccount.Status.ACTIVE), false); + session.setAttribute(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY, new SecurityContextImpl( + UsernamePasswordAuthenticationToken.authenticated(old, null, old.getAuthorities()))); + request.setSession(session); + String oldId = session.getId(); + + filter.doFilter(request, new MockHttpServletResponse(), mock(FilterChain.class)); + + assertThat(sessionUser(request).getUserAccount().getId()).isEqualTo(current); + assertThat(request.getSession(false).getId()).isNotEqualTo(oldId); + } + + @Test + void theSessionsOwnUser_isKeptAsIs() throws Exception { + UUID id = UUID.randomUUID(); + when(decoder.decode("good")).thenReturn(jwt(id, List.of("ROLE_USER"))); + when(users.findById(id)).thenReturn(Optional.of(account(id, UserAccount.Status.ACTIVE))); + MockHttpServletRequest request = linkStart("steam", "good"); + MockHttpSession session = new MockHttpSession(); + CatapultOAuth2User own = new CatapultOAuth2User(null, account(id, UserAccount.Status.ACTIVE), false); + SecurityContext context = new SecurityContextImpl( + UsernamePasswordAuthenticationToken.authenticated(own, null, own.getAuthorities())); + session.setAttribute(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY, context); + request.setSession(session); + String id0 = session.getId(); + + filter.doFilter(request, new MockHttpServletResponse(), mock(FilterChain.class)); + + assertThat(request.getSession(false).getId()).isEqualTo(id0); + assertThat(request.getSession(false).getAttribute(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY)) + .isSameAs(context); + } + + @Test + void anInvalidToken_anUnknownOrInactiveAccount_orNoToken_restoreNothing() throws Exception { + UUID inactive = UUID.randomUUID(); + UUID unknown = UUID.randomUUID(); + when(decoder.decode("bad")).thenThrow(new BadJwtException("expired")); + when(decoder.decode("not-a-uuid")).thenReturn( + Jwt.withTokenValue("t").header("alg", "HS256").subject("42").build()); + when(decoder.decode("inactive")).thenReturn(jwt(inactive, List.of("ROLE_USER"))); + when(decoder.decode("unknown")).thenReturn(jwt(unknown, List.of("ROLE_USER"))); + when(users.findById(inactive)).thenReturn(Optional.of(account(inactive, UserAccount.Status.INACTIVE))); + when(users.findById(unknown)).thenReturn(Optional.empty()); + + for (String token : new String[]{"bad", "not-a-uuid", "inactive", "unknown", null}) { + MockHttpServletRequest request = linkStart("battlenet", token); + FilterChain chain = mock(FilterChain.class); + MockHttpServletResponse response = new MockHttpServletResponse(); + filter.doFilter(request, response, chain); + assertThat(request.getSession(false)).as(String.valueOf(token)).isNull(); + verify(chain).doFilter(request, response); + } + } + + @Test + void theTwitchLogin_andOtherPaths_areLeftAlone() throws Exception { + for (String uri : new String[]{"/oauth2/authorization/twitch", "/login/oauth2/code/battlenet", "/api/x"}) { + MockHttpServletRequest request = new MockHttpServletRequest("GET", uri); + request.addHeader("Authorization", "Bearer good"); + filter.doFilter(request, new MockHttpServletResponse(), mock(FilterChain.class)); + assertThat(request.getSession(false)).as(uri).isNull(); + } + verifyNoInteractions(decoder); + verifyNoInteractions(users); + } +} diff --git a/catapult-web/src/main/java/fr/enimaloc/catapult/relay/ApiRelayController.java b/catapult-web/src/main/java/fr/enimaloc/catapult/relay/ApiRelayController.java index e86e498d3..e0cfc235e 100644 --- a/catapult-web/src/main/java/fr/enimaloc/catapult/relay/ApiRelayController.java +++ b/catapult-web/src/main/java/fr/enimaloc/catapult/relay/ApiRelayController.java @@ -2,6 +2,8 @@ import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; +import jakarta.servlet.http.HttpSession; +import fr.enimaloc.catapult.service.http.ApiClient; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; @@ -52,6 +54,7 @@ public class ApiRelayController { "/api/me/custom-bindings", "/api/me/custom-bindings/**", "/api/me/drops", "/api/games/**", "/api/v3/api-docs", "/api/v3/api-docs/**", }; + private static final String LINK_PREFIX = "/oauth2/authorization/"; /** A relayed request's body is read whole: these endpoints only take small ones. */ static final int MAX_BODY_BYTES = 2 * 1024 * 1024; static final Duration CONNECT_TIMEOUT = Duration.ofSeconds(5); @@ -139,11 +142,14 @@ HttpRequest upstream(HttpServletRequest request, byte[] body) { .method(request.getMethod(), body.length == 0 ? HttpRequest.BodyPublishers.noBody() : HttpRequest.BodyPublishers.ofByteArray(body)); + String linkJwt = linkJwt(request); for (String name : Collections.list(request.getHeaderNames())) { String lower = name.toLowerCase(Locale.ROOT); if (NOT_RELAYED.contains(lower) || FORWARDING.contains(lower)) continue; + if (linkJwt != null && lower.equals("authorization")) continue; for (String value : Collections.list(request.getHeaders(name))) upstream.header(name, value); } + if (linkJwt != null) upstream.header("Authorization", "Bearer " + linkJwt); return upstream .header("X-Forwarded-For", request.getRemoteAddr()) .header("X-Real-IP", request.getRemoteAddr()) @@ -153,6 +159,19 @@ HttpRequest upstream(HttpServletRequest request, byte[] body) { .build(); } + /** + * The logged-in user's JWT when a secondary link (Steam, Xbox, Battle.net) starts: catapult-api's session, which + * its callback reads the user from, may have been lost (restart, other replica), so catapult-api restores it + * from this token (its LinkSessionRestoreFilter). Null for any other request. + */ + private static String linkJwt(HttpServletRequest request) { + String uri = request.getRequestURI(); + if (!uri.startsWith(LINK_PREFIX) || uri.equals(LINK_PREFIX + "twitch")) return null; + HttpSession session = request.getSession(false); + return session != null && session.getAttribute(ApiClient.SESSION_JWT_KEY) instanceof String jwt + && !jwt.isBlank() ? jwt : null; + } + private void unavailable(HttpServletResponse response) throws IOException { response.setStatus(HttpStatus.SERVICE_UNAVAILABLE.value()); response.setHeader("Retry-After", "30"); diff --git a/catapult-web/src/test/java/fr/enimaloc/catapult/relay/ApiRelayControllerTest.java b/catapult-web/src/test/java/fr/enimaloc/catapult/relay/ApiRelayControllerTest.java index 0d3bbea11..72f488ee9 100644 --- a/catapult-web/src/test/java/fr/enimaloc/catapult/relay/ApiRelayControllerTest.java +++ b/catapult-web/src/test/java/fr/enimaloc/catapult/relay/ApiRelayControllerTest.java @@ -3,6 +3,7 @@ import com.sun.net.httpserver.HttpExchange; import com.sun.net.httpserver.HttpServer; import fr.enimaloc.catapult.security.WebSecurityConfig; +import fr.enimaloc.catapult.service.http.ApiClient; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -12,6 +13,7 @@ import jakarta.servlet.http.Cookie; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.mock.web.MockHttpSession; import org.springframework.test.context.DynamicPropertyRegistry; import org.springframework.test.context.DynamicPropertySource; import org.springframework.test.web.servlet.MockMvc; @@ -131,6 +133,33 @@ void relaysTheRequest_asTheVisitorSentIt_withTheForwardingCatapultWebVouchesFor( assertThat(received.header("Connection")).isNotEqualTo("keep-alive, X-Custom"); } + /** A secondary link's start carries the session's JWT: catapult-api restores its own session from it. */ + @Test + void secondaryLinkStart_carriesTheSessionJwt_inPlaceOfTheVisitorsAuthorization() throws Exception { + MockHttpSession session = new MockHttpSession(); + session.setAttribute(ApiClient.SESSION_JWT_KEY, "session-jwt"); + + for (String provider : List.of("battlenet", "xbox", "steam")) { + mvc.perform(get("/oauth2/authorization/" + provider).session(session) + .header("Authorization", "Bearer forged")) + .andExpect(status().isOk()); + assertThat(RECEIVED.get().header("Authorization")).as(provider).isEqualTo("Bearer session-jwt"); + } + } + + @Test + void theTwitchLogin_andOtherPaths_neverGetTheSessionJwt() throws Exception { + MockHttpSession session = new MockHttpSession(); + session.setAttribute(ApiClient.SESSION_JWT_KEY, "session-jwt"); + + mvc.perform(get("/oauth2/authorization/twitch").session(session)).andExpect(status().isOk()); + assertThat(RECEIVED.get().header("Authorization")).isNull(); + mvc.perform(get("/login/oauth2/code/battlenet").session(session)).andExpect(status().isOk()); + assertThat(RECEIVED.get().header("Authorization")).isNull(); + mvc.perform(get("/oauth2/authorization/battlenet")).andExpect(status().isOk()); + assertThat(RECEIVED.get().header("Authorization")).isNull(); + } + /** The login's redirect to Twitch, and the cookies it sets, must reach the browser untouched. */ @Test void relaysTheAnswer_redirectsAndCookiesIncluded_neverFollowingIt() throws Exception { From bd55b7c5acd0d78c5d14cd212cd4b1723067fb92 Mon Sep 17 00:00:00 2001 From: enimaloc Date: Sat, 14 Nov 2026 07:18:58 +0100 Subject: [PATCH 2/2] fix: never restore a link session while an admin impersonates a member An admin acting as a member must not link their own Steam, Xbox or Battle.net account to the member's: catapult-web sends no JWT then, and catapult-api refuses impersonation tokens. The filter also resolves its beans on use, so web test slices that pick it up still load. --- .../security/LinkSessionRestoreFilter.java | 21 +++++++++++++---- .../LinkSessionRestoreFilterTest.java | 23 ++++++++++++++++++- .../catapult/relay/ApiRelayController.java | 5 +++- .../relay/ApiRelayControllerTest.java | 12 ++++++++++ 4 files changed, 54 insertions(+), 7 deletions(-) diff --git a/catapult-api/src/main/java/fr/enimaloc/catapult/security/LinkSessionRestoreFilter.java b/catapult-api/src/main/java/fr/enimaloc/catapult/security/LinkSessionRestoreFilter.java index 60960a2e5..d10be1a5f 100644 --- a/catapult-api/src/main/java/fr/enimaloc/catapult/security/LinkSessionRestoreFilter.java +++ b/catapult-api/src/main/java/fr/enimaloc/catapult/security/LinkSessionRestoreFilter.java @@ -9,6 +9,7 @@ import jakarta.servlet.http.HttpSession; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; +import org.springframework.beans.factory.ObjectProvider; import org.springframework.core.Ordered; import org.springframework.core.annotation.Order; import org.springframework.http.HttpHeaders; @@ -46,9 +47,12 @@ public class LinkSessionRestoreFilter extends OncePerRequestFilter { static final String PATH_PREFIX = "/oauth2/authorization/"; private static final String BEARER = "Bearer "; + /** Carried by the JWT an admin gets to act as a member ({@link ImpersonationService}). */ + static final String IMPERSONATION_ROLE = "ROLE_PREVIOUS_ADMINISTRATOR"; - private final JwtDecoder jwtDecoder; - private final UserAccountRepository users; + // Resolved on use: a servlet filter is picked up by every web test slice, which has neither + private final ObjectProvider jwtDecoder; + private final ObjectProvider users; @Override protected boolean shouldNotFilter(HttpServletRequest request) { @@ -79,18 +83,25 @@ private static UUID sessionUserId(HttpSession session) { private Optional bearerUser(HttpServletRequest request) { String header = request.getHeader(HttpHeaders.AUTHORIZATION); - if (header == null || !header.startsWith(BEARER)) return Optional.empty(); + JwtDecoder decoder = jwtDecoder.getIfAvailable(); + UserAccountRepository accounts = users.getIfAvailable(); + if (header == null || !header.startsWith(BEARER) || decoder == null || accounts == null) return Optional.empty(); Jwt jwt; UUID id; try { - jwt = jwtDecoder.decode(header.substring(BEARER.length()).trim()); + jwt = decoder.decode(header.substring(BEARER.length()).trim()); id = UUID.fromString(jwt.getSubject()); } catch (JwtException | IllegalArgumentException | NullPointerException e) { log.info("Secondary link started with an unusable token ({})", e.getClass().getSimpleName()); return Optional.empty(); } List roles = Optional.ofNullable(jwt.getClaimAsStringList("roles")).orElse(List.of()); - return users.findById(id) + if (roles.contains(IMPERSONATION_ROLE)) { + // An admin acting as a member: their own provider account must never be linked to the member's + log.info("Secondary link started while impersonating: no session restored"); + return Optional.empty(); + } + return accounts.findById(id) .filter(account -> account.getStatus() == UserAccount.Status.ACTIVE) .map(account -> new CatapultOAuth2User(null, account, roles.contains("ROLE_ADMIN"))); } diff --git a/catapult-api/src/test/java/fr/enimaloc/catapult/security/LinkSessionRestoreFilterTest.java b/catapult-api/src/test/java/fr/enimaloc/catapult/security/LinkSessionRestoreFilterTest.java index 9fcae09de..13d28ed4a 100644 --- a/catapult-api/src/test/java/fr/enimaloc/catapult/security/LinkSessionRestoreFilterTest.java +++ b/catapult-api/src/test/java/fr/enimaloc/catapult/security/LinkSessionRestoreFilterTest.java @@ -4,6 +4,7 @@ import fr.enimaloc.catapult.repository.account.UserAccountRepository; import jakarta.servlet.FilterChain; import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.ObjectProvider; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.mock.web.MockHttpSession; @@ -32,7 +33,14 @@ class LinkSessionRestoreFilterTest { private final JwtDecoder decoder = mock(JwtDecoder.class); private final UserAccountRepository users = mock(UserAccountRepository.class); - private final LinkSessionRestoreFilter filter = new LinkSessionRestoreFilter(decoder, users); + private final LinkSessionRestoreFilter filter = new LinkSessionRestoreFilter(provider(decoder), provider(users)); + + @SuppressWarnings("unchecked") + private static ObjectProvider provider(T bean) { + ObjectProvider provider = mock(ObjectProvider.class); + when(provider.getIfAvailable()).thenReturn(bean); + return provider; + } private static UserAccount account(UUID id, UserAccount.Status status) { UserAccount account = new UserAccount(); @@ -143,6 +151,19 @@ void anInvalidToken_anUnknownOrInactiveAccount_orNoToken_restoreNothing() throws } } + @Test + void anImpersonationToken_restoresNothing() throws Exception { + UUID member = UUID.randomUUID(); + when(decoder.decode("impersonation")).thenReturn( + jwt(member, List.of("ROLE_USER", LinkSessionRestoreFilter.IMPERSONATION_ROLE))); + when(users.findById(member)).thenReturn(Optional.of(account(member, UserAccount.Status.ACTIVE))); + MockHttpServletRequest request = linkStart("battlenet", "impersonation"); + + filter.doFilter(request, new MockHttpServletResponse(), mock(FilterChain.class)); + + assertThat(request.getSession(false)).isNull(); + } + @Test void theTwitchLogin_andOtherPaths_areLeftAlone() throws Exception { for (String uri : new String[]{"/oauth2/authorization/twitch", "/login/oauth2/code/battlenet", "/api/x"}) { diff --git a/catapult-web/src/main/java/fr/enimaloc/catapult/relay/ApiRelayController.java b/catapult-web/src/main/java/fr/enimaloc/catapult/relay/ApiRelayController.java index e0cfc235e..efeb35aba 100644 --- a/catapult-web/src/main/java/fr/enimaloc/catapult/relay/ApiRelayController.java +++ b/catapult-web/src/main/java/fr/enimaloc/catapult/relay/ApiRelayController.java @@ -3,6 +3,7 @@ import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import jakarta.servlet.http.HttpSession; +import fr.enimaloc.catapult.controller.ImpersonationController; import fr.enimaloc.catapult.service.http.ApiClient; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; @@ -162,11 +163,13 @@ HttpRequest upstream(HttpServletRequest request, byte[] body) { /** * The logged-in user's JWT when a secondary link (Steam, Xbox, Battle.net) starts: catapult-api's session, which * its callback reads the user from, may have been lost (restart, other replica), so catapult-api restores it - * from this token (its LinkSessionRestoreFilter). Null for any other request. + * from this token (its LinkSessionRestoreFilter). Null for any other request, and while an admin impersonates a + * member: the admin's own account must never be linked to the member's. */ private static String linkJwt(HttpServletRequest request) { String uri = request.getRequestURI(); if (!uri.startsWith(LINK_PREFIX) || uri.equals(LINK_PREFIX + "twitch")) return null; + if (ImpersonationController.impersonating(request)) return null; HttpSession session = request.getSession(false); return session != null && session.getAttribute(ApiClient.SESSION_JWT_KEY) instanceof String jwt && !jwt.isBlank() ? jwt : null; diff --git a/catapult-web/src/test/java/fr/enimaloc/catapult/relay/ApiRelayControllerTest.java b/catapult-web/src/test/java/fr/enimaloc/catapult/relay/ApiRelayControllerTest.java index 72f488ee9..e6ce0323f 100644 --- a/catapult-web/src/test/java/fr/enimaloc/catapult/relay/ApiRelayControllerTest.java +++ b/catapult-web/src/test/java/fr/enimaloc/catapult/relay/ApiRelayControllerTest.java @@ -3,6 +3,7 @@ import com.sun.net.httpserver.HttpExchange; import com.sun.net.httpserver.HttpServer; import fr.enimaloc.catapult.security.WebSecurityConfig; +import fr.enimaloc.catapult.controller.ImpersonationController; import fr.enimaloc.catapult.service.http.ApiClient; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeEach; @@ -147,6 +148,17 @@ void secondaryLinkStart_carriesTheSessionJwt_inPlaceOfTheVisitorsAuthorization() } } + @Test + void anImpersonatingAdmin_neverSendsTheMembersJwt() throws Exception { + MockHttpSession session = new MockHttpSession(); + session.setAttribute(ApiClient.SESSION_JWT_KEY, "member-jwt"); + session.setAttribute(ImpersonationController.ORIGINAL_JWT_KEY, "admin-jwt"); + + mvc.perform(get("/oauth2/authorization/battlenet").session(session)).andExpect(status().isOk()); + + assertThat(RECEIVED.get().header("Authorization")).isNull(); + } + @Test void theTwitchLogin_andOtherPaths_neverGetTheSessionJwt() throws Exception { MockHttpSession session = new MockHttpSession();