-
Notifications
You must be signed in to change notification settings - Fork 0
Auth: wire Spring Security + Spring Session JDBC end to end #47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 07-09-auth_request-link_and_verify_flows
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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")); | ||
| }); | ||
| } | ||
|
|
||
| @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 { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
|
||
|
|
||
| .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.")); | ||
| } | ||
| } |
There was a problem hiding this comment.
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 theSecurityContextremains 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.Note that the
logout()method at line 86 correctly clears the context after invalidation.Spotted by Graphite

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