fix(sprint-0): panel-review-v2 blockers — scoped security chain, fail-closed CORS, no @ComponentScan, drop dead RefreshToken

Review-v2 (Sprint-0-Plan-Review-v2) blockers:
- B1: SecurityConfig chain now securityMatcher-scoped to plate-auth endpoints so it cannot hijack the consuming app's routes
- B2: removed @ComponentScan from auto-config; explicit @Import of @Configuration + @Service/@RestController classes
- B4: CORS fails closed (same-origin) when allowed-origins empty instead of defaulting to '*'
- B5: removed dead RefreshToken entity + repo; v0.1 uses stateless JWT refresh (rotation deferred to v0.3)
- W-A: documented OnboardingHook transaction contract

Verified: mvn -pl plate-auth-starter compile succeeds.
This commit is contained in:
Patrick Plate
2026-06-24 20:22:36 +02:00
parent 9d314a49c6
commit b43ab5e02c
6 changed files with 117 additions and 66 deletions
+15
View File
@@ -4,6 +4,21 @@ All notable changes to this project will be documented in this file.
## [Unreleased] ## [Unreleased]
### Security / Correctness — Review-v2 blockers fixed
- **B1:** `SecurityConfig` `SecurityFilterChain` is now `securityMatcher`-scoped to plate-auth's own
endpoints (`/api/auth/**`, `/api/invitations/**`, `/api/access-requests/**`, `/api/admin/**`, `/api/me`,
`/api/memberships/**`). Previously an unscoped `@Order(-100)` chain with `anyRequest().authenticated()`
would hijack the consuming app's own routes. (panel B1)
- **B2:** Removed `@ComponentScan(basePackages="de.platesoft.auth")` from `PlateAuthAutoConfiguration`
(auto-configuration anti-pattern per Spring Boot guidance). Replaced with explicit `@Import` of the
concrete `@Configuration` classes + `@Service`/`@RestController` components. (panel B2)
- **B4:** CORS now fails closed by default. Empty `plate.auth.cors.allowed-origins` disables CORS for
plate-auth endpoints (same-origin only) instead of defaulting to `allowedOriginPatterns("*")`. (panel B4)
- **B5:** Removed dead `RefreshToken` entity + `RefreshTokenRepository`. v0.1 issues stateless JWT refresh
tokens (per the documented threat model); rotation/family-tracking is deferred to v0.3. (panel B5)
- **W-A:** Documented the `OnboardingHook` transaction contract (hooks run inside the exchange
transaction; keep them fast + idempotent).
### Added ### Added
- Initial project scaffold (W1) - Initial project scaffold (W1)
- Maven parent POM with `${revision}` CI-friendly versioning - Maven parent POM with `${revision}` CI-friendly versioning
@@ -1,5 +1,13 @@
package de.platesoft.auth; package de.platesoft.auth;
import de.platesoft.auth.config.PlateAuthFlywayConfig;
import de.platesoft.auth.config.PlateAuthExceptionHandler;
import de.platesoft.auth.config.SecurityConfig;
import de.platesoft.auth.controller.OAuthController;
import de.platesoft.auth.service.ExchangeService;
import de.platesoft.auth.service.JwtService;
import de.platesoft.auth.service.LoginEventService;
import de.platesoft.auth.service.MembershipService;
import de.platesoft.auth.spi.*; import de.platesoft.auth.spi.*;
import de.platesoft.auth.spi.defaults.*; import de.platesoft.auth.spi.defaults.*;
import org.springframework.boot.autoconfigure.AutoConfiguration; import org.springframework.boot.autoconfigure.AutoConfiguration;
@@ -8,13 +16,38 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.boot.context.properties.EnableConfigurationProperties;
import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.ComponentScan; import org.springframework.context.annotation.Import;
import org.springframework.data.jpa.repository.config.EnableJpaRepositories; import org.springframework.data.jpa.repository.config.EnableJpaRepositories;
import org.springframework.scheduling.annotation.EnableAsync; import org.springframework.scheduling.annotation.EnableAsync;
/**
* plate-auth auto-configuration.
*
* <p><b>Review-v2 fix B2 — no {@code @ComponentScan}.</b> Spring Boot's auto-configuration guidance is
* explicit that starter auto-config classes must <em>not</em> use {@code @ComponentScan}. It previously
* swept the whole {@code de.platesoft.auth} package, which is unpredictable in a consumer and can
* double-instantiate beans. Instead we now {@link Import @Import} the concrete {@code @Configuration}
* classes explicitly ({@link SecurityConfig}, {@link PlateAuthFlywayConfig},
* {@link PlateAuthExceptionHandler}) and declare every SPI default as an explicit {@code @Bean} below.
*
* <p>Every concrete {@code @Service} / {@code @RestController} class the starter publishes is also
* explicitly {@link Import @Import}ed here so the starter is <em>self-sufficient</em>: it does not rely
* on the consuming application scanning {@code de.platesoft.auth}. New services/controllers added in
* later workstreams must be appended to the {@code @Import} list (this is the trade-off vs the old
* broad scan — it is intentional and keeps the bean surface explicit and predictable).
*/
@AutoConfiguration @AutoConfiguration
@EnableConfigurationProperties(PlateAuthProperties.class) @EnableConfigurationProperties(PlateAuthProperties.class)
@ComponentScan(basePackages = "de.platesoft.auth") @Import({
SecurityConfig.class,
PlateAuthFlywayConfig.class,
PlateAuthExceptionHandler.class,
ExchangeService.class,
JwtService.class,
LoginEventService.class,
MembershipService.class,
OAuthController.class
})
@AutoConfigurationPackage(basePackages = "de.platesoft.auth.entity") @AutoConfigurationPackage(basePackages = "de.platesoft.auth.entity")
@EnableJpaRepositories(basePackages = "de.platesoft.auth.repository") @EnableJpaRepositories(basePackages = "de.platesoft.auth.repository")
@EnableAsync @EnableAsync
@@ -3,6 +3,7 @@ package de.platesoft.auth.config;
import de.platesoft.auth.PlateAuthProperties; import de.platesoft.auth.PlateAuthProperties;
import de.platesoft.auth.filter.JwtAuthenticationFilter; import de.platesoft.auth.filter.JwtAuthenticationFilter;
import de.platesoft.auth.service.JwtService; import de.platesoft.auth.service.JwtService;
import lombok.extern.slf4j.Slf4j;
import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Configuration;
import org.springframework.core.annotation.Order; import org.springframework.core.annotation.Order;
@@ -20,12 +21,32 @@ import org.springframework.web.cors.UrlBasedCorsConfigurationSource;
import java.util.List; import java.util.List;
/** /**
* Security configuration for plate-auth. Registers a SecurityFilterChain * Security configuration for plate-auth.
* that handles JWT validation and allows public access to auth endpoints. *
* <p><b>Review-v2 fix B1 — scoped chain.</b> This {@link SecurityFilterChain} is bound with
* {@code securityMatcher(...)} to <em>only</em> the endpoints plate-auth owns. This is mandatory for a
* starter: an unscoped chain at a high priority (previously {@code @Order(-100)} with
* {@code anyRequest().authenticated()}) would otherwise hijack the consuming application's own
* {@code SecurityFilterChain} and override its public routes. Consumers keep their own default
* (catch-all) chain for every path outside {@link #PLATE_AUTH_PATHS}.
*/ */
@Slf4j
@Configuration @Configuration
public class SecurityConfig { public class SecurityConfig {
/**
* Paths owned by plate-auth. The library's {@link SecurityFilterChain} only matches these.
* Everything else falls through to the consuming application's own security configuration.
*/
public static final String[] PLATE_AUTH_PATHS = {
"/api/auth/**",
"/api/invitations/**",
"/api/access-requests/**",
"/api/admin/**",
"/api/me",
"/api/memberships/**"
};
@Bean @Bean
public PasswordEncoder plateAuthPasswordEncoder() { public PasswordEncoder plateAuthPasswordEncoder() {
return new BCryptPasswordEncoder(); return new BCryptPasswordEncoder();
@@ -36,13 +57,21 @@ public class SecurityConfig {
return new JwtAuthenticationFilter(jwtService); return new JwtAuthenticationFilter(jwtService);
} }
/**
* plate-auth's security chain, scoped to {@link #PLATE_AUTH_PATHS} only.
*
* <p>Carries an explicit {@code securityMatcher} so it never competes with the consuming app's
* own (catch-all) {@code SecurityFilterChain}. Consumers retain full control of every path
* plate-auth does not own.
*/
@Bean @Bean
@Order(-100) @Order(100)
public SecurityFilterChain plateAuthSecurityFilterChain( public SecurityFilterChain plateAuthSecurityFilterChain(
HttpSecurity http, HttpSecurity http,
JwtAuthenticationFilter jwtFilter, JwtAuthenticationFilter jwtFilter,
PlateAuthProperties props) throws Exception { PlateAuthProperties props) throws Exception {
http http
.securityMatcher(PLATE_AUTH_PATHS)
.csrf(AbstractHttpConfigurer::disable) .csrf(AbstractHttpConfigurer::disable)
.cors(cors -> cors.configurationSource(corsConfigurationSource(props))) .cors(cors -> cors.configurationSource(corsConfigurationSource(props)))
.sessionManagement(session -> session.sessionCreationPolicy(SessionCreationPolicy.STATELESS)) .sessionManagement(session -> session.sessionCreationPolicy(SessionCreationPolicy.STATELESS))
@@ -53,7 +82,8 @@ public class SecurityConfig {
"/api/auth/register", "/api/auth/register",
"/api/auth/refresh", "/api/auth/refresh",
"/api/auth/config", "/api/auth/config",
"/actuator/health" "/api/access-requests",
"/api/access-requests/**"
).permitAll() ).permitAll()
.requestMatchers("/api/admin/**").hasAuthority("ROLE_ADMIN") .requestMatchers("/api/admin/**").hasAuthority("ROLE_ADMIN")
.anyRequest().authenticated() .anyRequest().authenticated()
@@ -62,18 +92,28 @@ public class SecurityConfig {
return http.build(); return http.build();
} }
/**
* CORS configuration source.
*
* <p><b>Review-v2 fix B4 — fail closed by default.</b> When {@code plate.auth.cors.allowed-origins}
* is empty (the default), CORS is <em>disabled</em> for plate-auth's endpoints (same-origin only).
* An authentication library must never default to {@code allowedOriginPatterns("*")}. Consumers
* opt into cross-origin access by listing their origins explicitly.
*/
private CorsConfigurationSource corsConfigurationSource(PlateAuthProperties props) { private CorsConfigurationSource corsConfigurationSource(PlateAuthProperties props) {
CorsConfiguration config = new CorsConfiguration(); UrlBasedCorsConfigurationSource source = new UrlBasedCorsConfigurationSource();
if (props.getCors().getAllowedOrigins().isEmpty()) { if (props.getCors().getAllowedOrigins().isEmpty()) {
config.setAllowedOriginPatterns(List.of("*")); // Fail closed: register no CORS rule → Spring rejects cross-origin requests to auth paths.
} else { log.warn("plate.auth.cors.allowed-origins is empty — CORS disabled for plate-auth endpoints "
config.setAllowedOrigins(props.getCors().getAllowedOrigins()); + "(same-origin only). Set allowed-origins to enable cross-origin access.");
config.setAllowCredentials(true); return source;
} }
CorsConfiguration config = new CorsConfiguration();
config.setAllowedOrigins(props.getCors().getAllowedOrigins());
config.setAllowCredentials(true);
config.setAllowedMethods(List.of("GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS")); config.setAllowedMethods(List.of("GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"));
config.setAllowedHeaders(List.of("*")); config.setAllowedHeaders(List.of("*"));
config.setMaxAge(3600L); config.setMaxAge(3600L);
UrlBasedCorsConfigurationSource source = new UrlBasedCorsConfigurationSource();
source.registerCorsConfiguration("/**", config); source.registerCorsConfiguration("/**", config);
return source; return source;
} }
@@ -1,42 +0,0 @@
package de.platesoft.auth.entity;
import jakarta.persistence.*;
import lombok.*;
import org.hibernate.envers.Audited;
import java.time.Instant;
import java.util.UUID;
/**
* Refresh token entity for rotation tracking.
*/
@Entity
@Table(name = "refresh_tokens")
@Getter @Setter @NoArgsConstructor @AllArgsConstructor @Builder
public class RefreshToken {
@Id
@Column(columnDefinition = "uuid")
private UUID id;
@Column(name = "user_id", nullable = false, columnDefinition = "uuid")
private UUID userId;
@Column(nullable = false, unique = true, length = 255)
private String token;
@Column(name = "expires_at", nullable = false)
private Instant expiresAt;
@Column(nullable = false)
private boolean revoked;
@Column(name = "created_at", nullable = false, updatable = false)
private Instant createdAt;
@PrePersist
void prePersist() {
if (id == null) id = UUID.randomUUID();
if (createdAt == null) createdAt = Instant.now();
}
}
@@ -1,12 +0,0 @@
package de.platesoft.auth.repository;
import de.platesoft.auth.entity.RefreshToken;
import org.springframework.data.jpa.repository.JpaRepository;
import java.util.Optional;
import java.util.UUID;
public interface RefreshTokenRepository extends JpaRepository<RefreshToken, UUID> {
Optional<RefreshToken> findByToken(String token);
void deleteByUserId(UUID userId);
}
@@ -6,8 +6,25 @@ import de.platesoft.auth.entity.User;
/** /**
* Called on first and subsequent sign-ins. Consumers wire their T3 onboarding logic here. * Called on first and subsequent sign-ins. Consumers wire their T3 onboarding logic here.
* The default implementation is a no-op. * The default implementation is a no-op.
*
* <p><b>Transaction contract (Review-v2 W-A).</b> Both methods are invoked <em>inside</em> the
* {@code @Transactional} boundary of {@code ExchangeService.verifyAndExchange} — i.e. within the same
* transaction that creates/updates the user and records the login event. Two consequences consumers
* must respect:
* <ul>
* <li><b>Keep it fast and idempotent.</b> A slow hook blocks the login response; a failing hook
* rolls back the entire sign-in (user creation + login event). Do heavy/external work
* asynchronously, or move it to a post-commit listener, not inline here.</li>
* <li><b>Idempotency required.</b> The same identity may trigger {@code onFirstSignIn} more than
* once under retries/races; implementations must be safe to call repeatedly.</li>
* </ul>
* Future v0.2 may move these calls to a post-commit phase; until then assume "in-transaction".
*/ */
public interface OnboardingHook { public interface OnboardingHook {
/**
* Called the first time a user authenticates via a provider (no prior {@code user_identity}).
* Runs inside the exchange transaction — see class-level contract.
*/
void onFirstSignIn(User user, LoginProvider provider); void onFirstSignIn(User user, LoginProvider provider);
default void onSubsequentSignIn(User user, LoginProvider provider) { default void onSubsequentSignIn(User user, LoginProvider provider) {