Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 113 additions & 0 deletions src/main/java/org/patinanetwork/patchats/auth/AuthController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
package org.patinanetwork.patchats.auth;

import io.micrometer.core.annotation.Timed;
import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.tags.Tag;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import jakarta.servlet.http.HttpSession;
import jakarta.validation.Valid;
import java.util.List;
import lombok.RequiredArgsConstructor;
import org.patinanetwork.patchats.auth.dto.RequestLinkRequest;
import org.patinanetwork.patchats.auth.dto.SessionResponse;
import org.patinanetwork.patchats.auth.dto.VerifyRequest;
import org.patinanetwork.patchats.auth.repo.MemberAccount;
import org.patinanetwork.patchats.auth.repo.MemberAccountRepository;
import org.patinanetwork.patchats.auth.security.AuthenticatedMember;
import org.patinanetwork.patchats.common.dto.ApiResponder;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
import org.springframework.security.core.annotation.AuthenticationPrincipal;
import org.springframework.security.core.authority.SimpleGrantedAuthority;
import org.springframework.security.core.context.SecurityContext;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.security.web.context.SecurityContextRepository;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;

/** REST endpoints for magic-link sign-in and the current session. */
@RestController
@RequestMapping("/api")
@Tag(name = "Auth")
@Timed(value = "controller.execution")
@EnableConfigurationProperties(AuthProperties.class)
@RequiredArgsConstructor
public class AuthController {

/** The response is identical whether or not the email has an account, so nothing can be enumerated. */
private static final String GENERIC_REQUEST_MESSAGE = "Check your email for a sign-in link.";

private final AuthService authService;
private final MemberAccountRepository members;
private final SecurityContextRepository securityContextRepository;

@Operation(summary = "Email a single-use sign-in link")
@PostMapping("/auth/request-link")
public ResponseEntity<ApiResponder<Void>> requestLink(
@Valid @RequestBody final RequestLinkRequest request, final HttpServletRequest httpRequest) {
authService.requestLink(request.email(), httpRequest.getRemoteAddr());
return ResponseEntity.ok(ApiResponder.success(GENERIC_REQUEST_MESSAGE, null));
}

@Operation(summary = "Exchange a magic-link token for a session")
@PostMapping("/auth/verify")
public ResponseEntity<ApiResponder<SessionResponse>> verify(
@Valid @RequestBody final VerifyRequest request,
final HttpServletRequest httpRequest,
final HttpServletResponse httpResponse) {
final MemberAccount member = authService.verify(request.token());
login(member, httpRequest, httpResponse);
return ResponseEntity.ok(ApiResponder.success("Signed in.", SessionResponse.of(member)));
}

/** Reads the member fresh from the database so profile completion is never stale session state. */
@Operation(summary = "The currently signed-in member")
@GetMapping("/session")
public ResponseEntity<ApiResponder<SessionResponse>> session(
@AuthenticationPrincipal final AuthenticatedMember principal, final HttpServletRequest httpRequest) {
return members.findById(principal.memberId())
.map(account -> ResponseEntity.ok(ApiResponder.success("Signed in.", SessionResponse.of(account))))
.orElseGet(() -> {
invalidateSession(httpRequest);
return ResponseEntity.status(HttpStatus.UNAUTHORIZED).body(ApiResponder.failure("Not signed in"));
Comment on lines +76 to +78

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing SecurityContextHolder.clearContext() call when member not found. If a member is deleted from the database while their session is still active, the session gets invalidated but the SecurityContext remains in the thread-local for the duration of this request. This could allow subsequent code in the same request to still see an authenticated principal that should no longer exist.

.orElseGet(() -> {
    invalidateSession(httpRequest);
    SecurityContextHolder.clearContext();  // Add this line
    return ResponseEntity.status(HttpStatus.UNAUTHORIZED).body(ApiResponder.failure("Not signed in"));
});

Note that the logout() method at line 86 correctly clears the context after invalidation.

Suggested change
.orElseGet(() -> {
invalidateSession(httpRequest);
return ResponseEntity.status(HttpStatus.UNAUTHORIZED).body(ApiResponder.failure("Not signed in"));
.orElseGet(() -> {
invalidateSession(httpRequest);
SecurityContextHolder.clearContext();
return ResponseEntity.status(HttpStatus.UNAUTHORIZED).body(ApiResponder.failure("Not signed in"));

Spotted by Graphite

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

});
}

@Operation(summary = "Sign out")
@PostMapping("/auth/logout")
public ResponseEntity<ApiResponder<Void>> logout(final HttpServletRequest httpRequest) {
invalidateSession(httpRequest);
SecurityContextHolder.clearContext();
return ResponseEntity.ok(ApiResponder.success("Signed out.", null));
}

/**
* Programmatic login: store the authenticated principal in the {@link SecurityContextRepository}, which Spring
* Session persists and turns into the session cookie. {@code changeSessionId} rotates any pre-existing session so a
* client-supplied id can never survive into an authenticated session (fixation defense).
*/
private void login(
final MemberAccount member, final HttpServletRequest request, final HttpServletResponse response) {
if (request.getSession(false) != null) {
request.changeSessionId();
}
final SecurityContext context = SecurityContextHolder.createEmptyContext();
context.setAuthentication(UsernamePasswordAuthenticationToken.authenticated(
AuthenticatedMember.of(member), null, List.of(new SimpleGrantedAuthority("ROLE_MEMBER"))));
SecurityContextHolder.setContext(context);
securityContextRepository.saveContext(context, request, response);
}

private void invalidateSession(final HttpServletRequest request) {
final HttpSession session = request.getSession(false);
if (session != null) {
session.invalidate();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package org.patinanetwork.patchats.auth.security;

import com.fasterxml.jackson.databind.ObjectMapper;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import java.io.IOException;
import lombok.RequiredArgsConstructor;
import org.patinanetwork.patchats.common.dto.ApiResponder;
import org.springframework.http.MediaType;
import org.springframework.security.core.AuthenticationException;
import org.springframework.security.web.AuthenticationEntryPoint;
import org.springframework.stereotype.Component;

/** Answers unauthenticated requests to protected endpoints with a 401 in the standard JSON envelope. */
@Component
@RequiredArgsConstructor
public class ApiAuthenticationEntryPoint implements AuthenticationEntryPoint {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to add an example of how to protect an endpoint.


private final ObjectMapper objectMapper;

@Override
public void commence(
final HttpServletRequest request,
final HttpServletResponse response,
final AuthenticationException authException)
throws IOException {
response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
response.setContentType(MediaType.APPLICATION_JSON_VALUE);
objectMapper.writeValue(response.getWriter(), ApiResponder.failure("Not signed in"));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package org.patinanetwork.patchats.auth.security;

import java.io.Serializable;
import java.security.Principal;
import java.util.UUID;
import org.patinanetwork.patchats.auth.repo.MemberAccount;

/**
* The security principal stored in the session. Must stay {@link Serializable} (and small): Spring Session JDBC
* serializes the whole {@code SecurityContext} into {@code spring_session_attributes}. Implementing {@link Principal}
* gives {@code Authentication.getName()} — and Spring Session's {@code PRINCIPAL_NAME} index — the member's email.
*/
public record AuthenticatedMember(UUID memberId, String email) implements Principal, Serializable {

public static AuthenticatedMember of(final MemberAccount account) {
return new AuthenticatedMember(account.id(), account.email());
}

@Override
public String getName() {
return email;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
package org.patinanetwork.patchats.auth.security;

import java.time.Duration;
import lombok.RequiredArgsConstructor;
import org.patinanetwork.patchats.auth.AuthProperties;
import org.springframework.boot.autoconfigure.session.SessionProperties;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Profile;
import org.springframework.http.HttpMethod;
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
import org.springframework.security.config.annotation.web.configurers.AbstractHttpConfigurer;
import org.springframework.security.web.SecurityFilterChain;
import org.springframework.security.web.context.HttpSessionSecurityContextRepository;
import org.springframework.security.web.context.SecurityContextRepository;
import org.springframework.session.web.http.DefaultCookieSerializer;

/**
* Wires cookie-session authentication on top of Spring Session JDBC.
*
* <p><b>How a request is authenticated.</b> Spring Session's {@code SessionRepositoryFilter} resolves the
* {@code patchats_session} cookie into an {@code HttpSession} backed by the {@code spring_session} tables, and Spring
* Security's {@code SecurityContextHolderFilter} restores the {@link AuthenticatedMember} principal that
* {@code AuthController} saved at magic-link verification. There is no {@code JSESSIONID} and the server never adopts a
* client-supplied session id, so session fixation is prevented by construction (verify additionally rotates any
* pre-existing session id).
*
* <p><b>Why CSRF protection is disabled.</b> Three layers make the classic attack a no-op here: the session cookie is
* {@code SameSite=Lax}, so browsers do not attach it to cross-site POSTs; every state-changing endpoint only consumes
* {@code application/json} request bodies, which cross-site forms cannot produce and cross-origin {@code fetch} cannot
* send without a CORS preflight (and no CORS mappings exist — the SPA is served same-origin, via the Vite proxy in
* dev); and the one anonymous cookie-consuming POST, logout, is idempotent and harmless. Revisit if the cookie ever
* needs {@code SameSite=None}.
*/
@Configuration
@EnableConfigurationProperties({AuthProperties.class, SessionProperties.class})
@RequiredArgsConstructor
public class SecurityConfig {

public static final String SESSION_COOKIE_NAME = "patchats_session";

private final ApiAuthenticationEntryPoint authenticationEntryPoint;

/** Shapes the Spring Session cookie; picked up automatically by Spring Session's auto-configuration. */
@Bean
public DefaultCookieSerializer cookieSerializer(
final AuthProperties authProperties, final SessionProperties sessionProperties) {
final DefaultCookieSerializer serializer = new DefaultCookieSerializer();
serializer.setCookieName(SESSION_COOKIE_NAME);
serializer.setUseHttpOnlyCookie(true);
serializer.setUseSecureCookie(authProperties.isCookieSecure());
serializer.setSameSite("Lax");
serializer.setCookiePath("/");
// Persistent cookie matching the server-side inactivity timeout (spring.session.timeout).
final Duration timeout = sessionProperties.getTimeout();
serializer.setCookieMaxAge((int) timeout.toSeconds());
return serializer;
}

/** Shared by the filter chain (restore on request) and {@code AuthController} (save on login). */
@Bean
public SecurityContextRepository securityContextRepository() {
return new HttpSessionSecurityContextRepository();
}

/**
* Default/production chain.
*
* <p>NOTE: the admin role is not yet assigned anywhere, so the email rule still fails closed — every caller is
* denied until an admin domain lands. Other endpoints keep their prior open posture.
*/
@Bean
@Profile("!dev")
SecurityFilterChain securityFilterChain(final HttpSecurity http) throws Exception {
return common(http)
.authorizeHttpRequests(auth -> auth.requestMatchers(HttpMethod.POST, "/api/email/**")
.hasRole("ADMIN")
.requestMatchers(HttpMethod.GET, "/api/session")
.authenticated()
.anyRequest()
.permitAll())
.build();
}

/** Local-dev chain: only the session endpoint needs auth so the login flow can be exercised end to end. */
@Bean
@Profile("dev")
SecurityFilterChain devSecurityFilterChain(final HttpSecurity http) throws Exception {
return common(http)
.authorizeHttpRequests(auth -> auth.requestMatchers(HttpMethod.GET, "/api/session")
.authenticated()
.anyRequest()
.permitAll())
.build();
}

private HttpSecurity common(final HttpSecurity http) throws Exception {
return http.csrf(AbstractHttpConfigurer::disable)

Check failure on line 99 in src/main/java/org/patinanetwork/patchats/auth/security/SecurityConfig.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Make sure disabling Spring Security's CSRF protection is safe here.

See more on https://sonarcloud.io/project/issues?id=Patina-Network_patchats&issues=AZ9I16wqZ9F0_slUd7MQ&open=AZ9I16wqZ9F0_slUd7MQ&pullRequest=47

Check failure

Code scanning / SonarCloud

CSRF protections should not be disabled High

Make sure disabling Spring Security's CSRF protection is safe here. See more on SonarQube Cloud
.requestCache(AbstractHttpConfigurer::disable)
.logout(AbstractHttpConfigurer::disable)
.securityContext(context -> context.securityContextRepository(securityContextRepository()))
.exceptionHandling(handling -> handling.authenticationEntryPoint(authenticationEntryPoint));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
package org.patinanetwork.patchats.auth;

import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;

import java.time.OffsetDateTime;
import java.time.ZoneOffset;
import java.util.UUID;
import org.junit.jupiter.api.Test;
import org.patinanetwork.patchats.auth.repo.MemberAccount;
import org.patinanetwork.patchats.auth.repo.MemberAccountRepository;
import org.patinanetwork.patchats.common.web.ApiExceptionHandler;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest;
import org.springframework.context.annotation.Import;
import org.springframework.http.MediaType;
import org.springframework.security.web.context.SecurityContextRepository;
import org.springframework.test.context.bean.override.mockito.MockitoBean;
import org.springframework.test.web.servlet.MockMvc;

@WebMvcTest(AuthController.class)
@AutoConfigureMockMvc(addFilters = false)
@Import(ApiExceptionHandler.class)
class AuthControllerTest {

@Autowired
private MockMvc mockMvc;

@MockitoBean
private AuthService authService;

@MockitoBean
private MemberAccountRepository members;

@MockitoBean
private SecurityContextRepository securityContextRepository;

@Test
void requestLinkAlwaysReturnsGenericMessage() throws Exception {
mockMvc.perform(post("/api/auth/request-link")
.contentType(MediaType.APPLICATION_JSON)
.content("{\"email\":\"ann@example.com\"}"))
.andExpect(status().isOk())
.andExpect(jsonPath("$.success").value(true))
.andExpect(jsonPath("$.message").value("Check your email for a sign-in link."));

verify(authService).requestLink(eq("ann@example.com"), anyString());
}

@Test
void requestLinkRejectsMalformedEmail() throws Exception {
mockMvc.perform(post("/api/auth/request-link")
.contentType(MediaType.APPLICATION_JSON)
.content("{\"email\":\"not-an-email\"}"))
.andExpect(status().isBadRequest())
.andExpect(jsonPath("$.success").value(false));
}

@Test
void verifyReturnsSessionPayloadAndSavesContext() throws Exception {
final MemberAccount member =
new MemberAccount(UUID.randomUUID(), "ann@example.com", "Ann", OffsetDateTime.now(ZoneOffset.UTC));
when(authService.verify("raw-token")).thenReturn(member);

mockMvc.perform(post("/api/auth/verify")
.contentType(MediaType.APPLICATION_JSON)
.content("{\"token\":\"raw-token\"}"))
.andExpect(status().isOk())
.andExpect(jsonPath("$.success").value(true))
.andExpect(jsonPath("$.payload.email").value("ann@example.com"))
.andExpect(jsonPath("$.payload.name").value("Ann"))
.andExpect(jsonPath("$.payload.isAdmin").value(false))
.andExpect(jsonPath("$.payload.profileCompleted").value(true));

verify(securityContextRepository).saveContext(any(), any(), any());
}

@Test
void verifyShellMemberHasNullNameAndIncompleteProfile() throws Exception {
when(authService.verify("raw-token"))
.thenReturn(new MemberAccount(UUID.randomUUID(), "new@example.com", null, null));

mockMvc.perform(post("/api/auth/verify")
.contentType(MediaType.APPLICATION_JSON)
.content("{\"token\":\"raw-token\"}"))
.andExpect(status().isOk())
.andExpect(jsonPath("$.payload.name").doesNotExist())
.andExpect(jsonPath("$.payload.profileCompleted").value(false));
}

@Test
void verifyMapsInvalidTokenToBadRequestEnvelope() throws Exception {
when(authService.verify("spent")).thenThrow(new InvalidMagicLinkException());

mockMvc.perform(post("/api/auth/verify")
.contentType(MediaType.APPLICATION_JSON)
.content("{\"token\":\"spent\"}"))
.andExpect(status().isBadRequest())
.andExpect(jsonPath("$.success").value(false))
.andExpect(
jsonPath("$.message").value("This sign-in link is invalid or has expired. Request a new one."));
}

@Test
void logoutIsIdempotent() throws Exception {
mockMvc.perform(post("/api/auth/logout"))
.andExpect(status().isOk())
.andExpect(jsonPath("$.success").value(true))
.andExpect(jsonPath("$.message").value("Signed out."));
}
}
Loading