-
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,121 @@ | ||
| 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.api.member.db.models.Member; | ||
| import org.patinanetwork.patchats.api.member.db.repos.MemberRepo; | ||
| 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.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 MemberRepo 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 Member 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 a deleted member is never served from stale session state; in that | ||
| * case the session is torn down (row invalidated and the thread-local context cleared, so nothing later in this | ||
| * request still sees an authenticated principal). | ||
| * | ||
| * <p>Looks up by email rather than id because {@code MemberRepo.getMemberById} is still | ||
| * {@code UnsupportedOperationException} — switch to it once the member domain implements it. The email in the | ||
| * principal is the value read straight off the member row at sign-in, so it matches exactly. | ||
| */ | ||
| @Operation(summary = "The currently signed-in member") | ||
| @GetMapping("/session") | ||
| public ResponseEntity<ApiResponder<SessionResponse>> session( | ||
| @AuthenticationPrincipal final AuthenticatedMember principal, final HttpServletRequest httpRequest) { | ||
| return members.getMemberByEmail(principal.email()) | ||
| .map(account -> ResponseEntity.ok(ApiResponder.success("Signed in.", SessionResponse.of(account)))) | ||
| .orElseGet(() -> { | ||
| invalidateSession(httpRequest); | ||
| SecurityContextHolder.clearContext(); | ||
| 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 Member 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,44 @@ | ||
| 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. | ||
| * | ||
| * <p>To protect an endpoint (i.e. make it trigger this entry point when no session cookie is presented), add a rule for | ||
| * it in {@link SecurityConfig}'s {@code authorizeHttpRequests} block: | ||
| * | ||
| * <pre>{@code | ||
| * .authorizeHttpRequests(auth -> auth | ||
| * .requestMatchers(HttpMethod.GET, "/api/matches/**").authenticated() // members only | ||
| * .anyRequest().permitAll()) | ||
| * }</pre> | ||
| * | ||
| * The controller can then read the signed-in member via {@code @AuthenticationPrincipal AuthenticatedMember}. | ||
| */ | ||
| @Component | ||
| @RequiredArgsConstructor | ||
| public class ApiAuthenticationEntryPoint implements AuthenticationEntryPoint { | ||
|
Member
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.
Contributor
Author
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. Added to the |
||
|
|
||
| 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.api.member.db.models.Member; | ||
|
|
||
| /** | ||
| * 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 Member member) { | ||
| return new AuthenticatedMember(member.getId(), member.getEmail()); | ||
| } | ||
|
|
||
| @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) | ||
|
github-advanced-security[bot] marked this conversation as resolved.
Fixed
|
||
| .requestCache(AbstractHttpConfigurer::disable) | ||
| .logout(AbstractHttpConfigurer::disable) | ||
| .securityContext(context -> context.securityContextRepository(securityContextRepository())) | ||
| .exceptionHandling(handling -> handling.authenticationEntryPoint(authenticationEntryPoint)); | ||
| } | ||
| } | ||
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.