feat(auth): delete AuthTokenCookieFilter and its test (ADR-020)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Marcel
@@ -1,137 +0,0 @@
|
|||||||
package org.raddatz.familienarchiv.security;
|
|
||||||
|
|
||||||
import jakarta.servlet.FilterChain;
|
|
||||||
import jakarta.servlet.ServletException;
|
|
||||||
import jakarta.servlet.http.Cookie;
|
|
||||||
import jakarta.servlet.http.HttpServletRequest;
|
|
||||||
import jakarta.servlet.http.HttpServletRequestWrapper;
|
|
||||||
import jakarta.servlet.http.HttpServletResponse;
|
|
||||||
import org.springframework.core.annotation.Order;
|
|
||||||
import org.springframework.http.HttpHeaders;
|
|
||||||
import org.springframework.stereotype.Component;
|
|
||||||
import org.springframework.web.filter.OncePerRequestFilter;
|
|
||||||
|
|
||||||
import java.io.IOException;
|
|
||||||
import java.net.URLDecoder;
|
|
||||||
import java.nio.charset.StandardCharsets;
|
|
||||||
import java.util.Collections;
|
|
||||||
import java.util.Enumeration;
|
|
||||||
|
|
||||||
/**
|
|
||||||
* Promotes the {@code auth_token} cookie to an {@code Authorization} header
|
|
||||||
* so that browser-side requests to {@code /api/*} authenticate the same way
|
|
||||||
* SSR fetches do.
|
|
||||||
*
|
|
||||||
* <p>The SvelteKit login action stores the full HTTP Basic header value
|
|
||||||
* ({@code "Basic <base64>"}) in an HttpOnly cookie. SSR fetches from
|
|
||||||
* {@code hooks.server.ts} read the cookie and pass it explicitly as the
|
|
||||||
* {@code Authorization} header. In the dev environment, Vite's proxy does
|
|
||||||
* the same on every {@code /api/*} request (see {@code vite.config.ts}).
|
|
||||||
* In production, Caddy proxies {@code /api/*} straight to the backend and
|
|
||||||
* does NOT translate the cookie — so client-side {@code fetch} and
|
|
||||||
* {@code EventSource} calls reach the backend without auth, get
|
|
||||||
* {@code 401 WWW-Authenticate: Basic}, and the browser pops a native dialog.
|
|
||||||
*
|
|
||||||
* <p>This filter closes that gap: if a request has an {@code auth_token}
|
|
||||||
* cookie but no explicit {@code Authorization} header, promote the cookie
|
|
||||||
* value (URL-decoded) into the header before Spring Security inspects it.
|
|
||||||
* Explicit {@code Authorization} headers are preserved unchanged.
|
|
||||||
*
|
|
||||||
* <p>See #520. Filter runs at {@code Ordered.HIGHEST_PRECEDENCE} so it
|
|
||||||
* mutates the request before any Spring Security filter sees it.
|
|
||||||
*
|
|
||||||
* <p><b>Scope:</b> only {@code /api/*} requests are touched. The
|
|
||||||
* {@code /actuator/*} block in Caddy plus the open auth/reset paths in
|
|
||||||
* {@link SecurityConfig} must NOT receive a promoted Authorization.
|
|
||||||
*
|
|
||||||
* <p><b>⚠ Log-leakage warning:</b> the wrapped request exposes the
|
|
||||||
* Authorization header via {@code getHeaderNames}/{@code getHeaders}. Any
|
|
||||||
* filter or interceptor that iterates request headers will see the live
|
|
||||||
* Basic credential. Do NOT add a request-header logger downstream of this
|
|
||||||
* filter without explicitly scrubbing the {@code Authorization} field.
|
|
||||||
*/
|
|
||||||
@Component
|
|
||||||
@Order(org.springframework.core.Ordered.HIGHEST_PRECEDENCE)
|
|
||||||
public class AuthTokenCookieFilter extends OncePerRequestFilter {
|
|
||||||
|
|
||||||
static final String COOKIE_NAME = "auth_token";
|
|
||||||
static final String SCOPE_PREFIX = "/api/";
|
|
||||||
|
|
||||||
@Override
|
|
||||||
protected void doFilterInternal(HttpServletRequest request,
|
|
||||||
HttpServletResponse response,
|
|
||||||
FilterChain chain) throws ServletException, IOException {
|
|
||||||
// Scope: only /api/* needs cookie promotion. /actuator/health (open),
|
|
||||||
// /api/auth/forgot-password (open), /login etc. don't.
|
|
||||||
if (!request.getRequestURI().startsWith(SCOPE_PREFIX)) {
|
|
||||||
chain.doFilter(request, response);
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
// An explicit Authorization header wins — this is the SSR fetch path
|
|
||||||
// (hooks.server.ts builds the header itself).
|
|
||||||
if (request.getHeader(HttpHeaders.AUTHORIZATION) != null) {
|
|
||||||
chain.doFilter(request, response);
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
Cookie[] cookies = request.getCookies();
|
|
||||||
if (cookies == null) {
|
|
||||||
chain.doFilter(request, response);
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
for (Cookie c : cookies) {
|
|
||||||
if (COOKIE_NAME.equals(c.getName()) && c.getValue() != null && !c.getValue().isBlank()) {
|
|
||||||
String decoded;
|
|
||||||
try {
|
|
||||||
decoded = URLDecoder.decode(c.getValue(), StandardCharsets.UTF_8);
|
|
||||||
} catch (IllegalArgumentException malformed) {
|
|
||||||
// Malformed percent-encoding — refuse to forward a bogus
|
|
||||||
// Authorization header. Spring Security will treat the
|
|
||||||
// request as unauthenticated.
|
|
||||||
chain.doFilter(request, response);
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
chain.doFilter(new AuthHeaderRequest(request, decoded), response);
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
chain.doFilter(request, response);
|
|
||||||
}
|
|
||||||
|
|
||||||
/**
|
|
||||||
* Adds (or overrides) the {@code Authorization} header on a wrapped request.
|
|
||||||
* All other headers pass through unchanged.
|
|
||||||
*/
|
|
||||||
static final class AuthHeaderRequest extends HttpServletRequestWrapper {
|
|
||||||
private final String authorization;
|
|
||||||
|
|
||||||
AuthHeaderRequest(HttpServletRequest request, String authorization) {
|
|
||||||
super(request);
|
|
||||||
this.authorization = authorization;
|
|
||||||
}
|
|
||||||
|
|
||||||
@Override
|
|
||||||
public String getHeader(String name) {
|
|
||||||
if (HttpHeaders.AUTHORIZATION.equalsIgnoreCase(name)) {
|
|
||||||
return authorization;
|
|
||||||
}
|
|
||||||
return super.getHeader(name);
|
|
||||||
}
|
|
||||||
|
|
||||||
@Override
|
|
||||||
public Enumeration<String> getHeaders(String name) {
|
|
||||||
if (HttpHeaders.AUTHORIZATION.equalsIgnoreCase(name)) {
|
|
||||||
return Collections.enumeration(Collections.singletonList(authorization));
|
|
||||||
}
|
|
||||||
return super.getHeaders(name);
|
|
||||||
}
|
|
||||||
|
|
||||||
@Override
|
|
||||||
public Enumeration<String> getHeaderNames() {
|
|
||||||
Enumeration<String> base = super.getHeaderNames();
|
|
||||||
java.util.Set<String> names = new java.util.LinkedHashSet<>();
|
|
||||||
while (base.hasMoreElements()) names.add(base.nextElement());
|
|
||||||
names.add(HttpHeaders.AUTHORIZATION);
|
|
||||||
return Collections.enumeration(names);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
@@ -1,134 +0,0 @@
|
|||||||
package org.raddatz.familienarchiv.security;
|
|
||||||
|
|
||||||
import jakarta.servlet.FilterChain;
|
|
||||||
import jakarta.servlet.http.Cookie;
|
|
||||||
import jakarta.servlet.http.HttpServletRequest;
|
|
||||||
import jakarta.servlet.http.HttpServletResponse;
|
|
||||||
import org.junit.jupiter.api.Test;
|
|
||||||
import org.mockito.ArgumentCaptor;
|
|
||||||
import org.springframework.mock.web.MockHttpServletRequest;
|
|
||||||
import org.springframework.mock.web.MockHttpServletResponse;
|
|
||||||
|
|
||||||
import static org.assertj.core.api.Assertions.assertThat;
|
|
||||||
import static org.mockito.Mockito.mock;
|
|
||||||
import static org.mockito.Mockito.times;
|
|
||||||
import static org.mockito.Mockito.verify;
|
|
||||||
|
|
||||||
/**
|
|
||||||
* The filter must turn a browser-side {@code Cookie: auth_token=Basic%20<base64>}
|
|
||||||
* into {@code Authorization: Basic <base64>} (URL-decoded) so that Spring's
|
|
||||||
* Basic-auth filter accepts it. Skips when the request already has an explicit
|
|
||||||
* {@code Authorization} header, or when no {@code auth_token} cookie is present.
|
|
||||||
*
|
|
||||||
* <p>See #520.
|
|
||||||
*/
|
|
||||||
class AuthTokenCookieFilterTest {
|
|
||||||
|
|
||||||
private final AuthTokenCookieFilter filter = new AuthTokenCookieFilter();
|
|
||||||
|
|
||||||
@Test
|
|
||||||
void promotes_url_encoded_auth_token_cookie_to_decoded_Authorization_header() throws Exception {
|
|
||||||
MockHttpServletRequest req = new MockHttpServletRequest();
|
|
||||||
req.setRequestURI("/api/users/me");
|
|
||||||
req.setCookies(new Cookie("auth_token", "Basic%20YWRtaW5AZmFtaWx5YXJjaGl2ZS5sb2NhbDpzZWNyZXQ%3D"));
|
|
||||||
MockHttpServletResponse res = new MockHttpServletResponse();
|
|
||||||
FilterChain chain = mock(FilterChain.class);
|
|
||||||
|
|
||||||
filter.doFilter(req, res, chain);
|
|
||||||
|
|
||||||
ArgumentCaptor<HttpServletRequest> captor = ArgumentCaptor.forClass(HttpServletRequest.class);
|
|
||||||
verify(chain, times(1)).doFilter(captor.capture(), org.mockito.ArgumentMatchers.any(HttpServletResponse.class));
|
|
||||||
|
|
||||||
HttpServletRequest forwarded = captor.getValue();
|
|
||||||
assertThat(forwarded.getHeader("Authorization"))
|
|
||||||
.as("Authorization must be URL-decoded so Spring's Basic parser sees a literal space")
|
|
||||||
.isEqualTo("Basic YWRtaW5AZmFtaWx5YXJjaGl2ZS5sb2NhbDpzZWNyZXQ=");
|
|
||||||
}
|
|
||||||
|
|
||||||
@Test
|
|
||||||
void preserves_explicit_Authorization_header_and_ignores_cookie() throws Exception {
|
|
||||||
MockHttpServletRequest req = new MockHttpServletRequest();
|
|
||||||
req.setRequestURI("/api/users/me");
|
|
||||||
req.addHeader("Authorization", "Basic explicit-header-wins");
|
|
||||||
req.setCookies(new Cookie("auth_token", "Basic%20cookie-would-have-promoted"));
|
|
||||||
MockHttpServletResponse res = new MockHttpServletResponse();
|
|
||||||
FilterChain chain = mock(FilterChain.class);
|
|
||||||
|
|
||||||
filter.doFilter(req, res, chain);
|
|
||||||
|
|
||||||
// Forwards the original request unchanged — same instance, no wrapping.
|
|
||||||
verify(chain).doFilter(req, res);
|
|
||||||
}
|
|
||||||
|
|
||||||
@Test
|
|
||||||
void passes_through_when_no_cookies_at_all() throws Exception {
|
|
||||||
MockHttpServletRequest req = new MockHttpServletRequest();
|
|
||||||
req.setRequestURI("/api/users/me");
|
|
||||||
MockHttpServletResponse res = new MockHttpServletResponse();
|
|
||||||
FilterChain chain = mock(FilterChain.class);
|
|
||||||
|
|
||||||
filter.doFilter(req, res, chain);
|
|
||||||
|
|
||||||
verify(chain).doFilter(req, res);
|
|
||||||
}
|
|
||||||
|
|
||||||
@Test
|
|
||||||
void passes_through_when_auth_token_cookie_is_absent() throws Exception {
|
|
||||||
MockHttpServletRequest req = new MockHttpServletRequest();
|
|
||||||
req.setRequestURI("/api/users/me");
|
|
||||||
req.setCookies(new Cookie("some_other_cookie", "value"));
|
|
||||||
MockHttpServletResponse res = new MockHttpServletResponse();
|
|
||||||
FilterChain chain = mock(FilterChain.class);
|
|
||||||
|
|
||||||
filter.doFilter(req, res, chain);
|
|
||||||
|
|
||||||
verify(chain).doFilter(req, res);
|
|
||||||
}
|
|
||||||
|
|
||||||
@Test
|
|
||||||
void passes_through_when_auth_token_cookie_is_empty() throws Exception {
|
|
||||||
MockHttpServletRequest req = new MockHttpServletRequest();
|
|
||||||
req.setRequestURI("/api/users/me");
|
|
||||||
req.setCookies(new Cookie("auth_token", ""));
|
|
||||||
MockHttpServletResponse res = new MockHttpServletResponse();
|
|
||||||
FilterChain chain = mock(FilterChain.class);
|
|
||||||
|
|
||||||
filter.doFilter(req, res, chain);
|
|
||||||
|
|
||||||
verify(chain).doFilter(req, res);
|
|
||||||
}
|
|
||||||
|
|
||||||
@Test
|
|
||||||
void passes_through_unchanged_when_request_is_outside_api_scope() throws Exception {
|
|
||||||
MockHttpServletRequest req = new MockHttpServletRequest();
|
|
||||||
// /actuator/health and similar must NOT receive a promoted Authorization
|
|
||||||
// header — they have their own access rules and should never be authed
|
|
||||||
// via the cookie.
|
|
||||||
req.setRequestURI("/actuator/health");
|
|
||||||
req.setCookies(new Cookie("auth_token", "Basic%20YWR=="));
|
|
||||||
MockHttpServletResponse res = new MockHttpServletResponse();
|
|
||||||
FilterChain chain = mock(FilterChain.class);
|
|
||||||
|
|
||||||
filter.doFilter(req, res, chain);
|
|
||||||
|
|
||||||
// Forwards the original request unchanged — same instance, no wrapping.
|
|
||||||
verify(chain).doFilter(req, res);
|
|
||||||
}
|
|
||||||
|
|
||||||
@Test
|
|
||||||
void passes_through_unchanged_when_cookie_value_is_malformed_percent_encoding() throws Exception {
|
|
||||||
MockHttpServletRequest req = new MockHttpServletRequest();
|
|
||||||
req.setRequestURI("/api/users/me");
|
|
||||||
// Lone "%" without two hex digits → URLDecoder throws → filter must
|
|
||||||
// refuse to forward a bogus Authorization header.
|
|
||||||
req.setCookies(new Cookie("auth_token", "Basic%2"));
|
|
||||||
MockHttpServletResponse res = new MockHttpServletResponse();
|
|
||||||
FilterChain chain = mock(FilterChain.class);
|
|
||||||
|
|
||||||
filter.doFilter(req, res, chain);
|
|
||||||
|
|
||||||
// Forwards the original request unchanged — Spring Security treats it
|
|
||||||
// as unauthenticated rather than crashing on bad input.
|
|
||||||
verify(chain).doFilter(req, res);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
Reference in New Issue
Block a user