diff --git a/CHANGELOG.md b/CHANGELOG.md index abd0502..849a0cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,21 @@ All notable changes to this project will be documented in this file. ## [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 - Initial project scaffold (W1) - Maven parent POM with `${revision}` CI-friendly versioning diff --git a/plate-auth-starter/src/main/java/de/platesoft/auth/PlateAuthAutoConfiguration.java b/plate-auth-starter/src/main/java/de/platesoft/auth/PlateAuthAutoConfiguration.java index babb444..fd92ee6 100644 --- a/plate-auth-starter/src/main/java/de/platesoft/auth/PlateAuthAutoConfiguration.java +++ b/plate-auth-starter/src/main/java/de/platesoft/auth/PlateAuthAutoConfiguration.java @@ -1,5 +1,13 @@ 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.defaults.*; 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.context.properties.EnableConfigurationProperties; 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.scheduling.annotation.EnableAsync; +/** + * plate-auth auto-configuration. + * + *
Review-v2 fix B2 — no {@code @ComponentScan}. Spring Boot's auto-configuration guidance is + * explicit that starter auto-config classes must not 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. + * + *
Every concrete {@code @Service} / {@code @RestController} class the starter publishes is also + * explicitly {@link Import @Import}ed here so the starter is self-sufficient: 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 @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") @EnableJpaRepositories(basePackages = "de.platesoft.auth.repository") @EnableAsync diff --git a/plate-auth-starter/src/main/java/de/platesoft/auth/config/SecurityConfig.java b/plate-auth-starter/src/main/java/de/platesoft/auth/config/SecurityConfig.java index cf7104c..030dde1 100644 --- a/plate-auth-starter/src/main/java/de/platesoft/auth/config/SecurityConfig.java +++ b/plate-auth-starter/src/main/java/de/platesoft/auth/config/SecurityConfig.java @@ -3,6 +3,7 @@ package de.platesoft.auth.config; import de.platesoft.auth.PlateAuthProperties; import de.platesoft.auth.filter.JwtAuthenticationFilter; import de.platesoft.auth.service.JwtService; +import lombok.extern.slf4j.Slf4j; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.core.annotation.Order; @@ -20,12 +21,32 @@ import org.springframework.web.cors.UrlBasedCorsConfigurationSource; import java.util.List; /** - * Security configuration for plate-auth. Registers a SecurityFilterChain - * that handles JWT validation and allows public access to auth endpoints. + * Security configuration for plate-auth. + * + *
Review-v2 fix B1 — scoped chain. This {@link SecurityFilterChain} is bound with + * {@code securityMatcher(...)} to only 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 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 public PasswordEncoder plateAuthPasswordEncoder() { return new BCryptPasswordEncoder(); @@ -36,13 +57,21 @@ public class SecurityConfig { return new JwtAuthenticationFilter(jwtService); } + /** + * plate-auth's security chain, scoped to {@link #PLATE_AUTH_PATHS} only. + * + *
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 - @Order(-100) + @Order(100) public SecurityFilterChain plateAuthSecurityFilterChain( HttpSecurity http, JwtAuthenticationFilter jwtFilter, PlateAuthProperties props) throws Exception { http + .securityMatcher(PLATE_AUTH_PATHS) .csrf(AbstractHttpConfigurer::disable) .cors(cors -> cors.configurationSource(corsConfigurationSource(props))) .sessionManagement(session -> session.sessionCreationPolicy(SessionCreationPolicy.STATELESS)) @@ -53,7 +82,8 @@ public class SecurityConfig { "/api/auth/register", "/api/auth/refresh", "/api/auth/config", - "/actuator/health" + "/api/access-requests", + "/api/access-requests/**" ).permitAll() .requestMatchers("/api/admin/**").hasAuthority("ROLE_ADMIN") .anyRequest().authenticated() @@ -62,18 +92,28 @@ public class SecurityConfig { return http.build(); } + /** + * CORS configuration source. + * + *
Review-v2 fix B4 — fail closed by default. When {@code plate.auth.cors.allowed-origins}
+ * is empty (the default), CORS is disabled 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) {
- CorsConfiguration config = new CorsConfiguration();
+ UrlBasedCorsConfigurationSource source = new UrlBasedCorsConfigurationSource();
if (props.getCors().getAllowedOrigins().isEmpty()) {
- config.setAllowedOriginPatterns(List.of("*"));
- } else {
- config.setAllowedOrigins(props.getCors().getAllowedOrigins());
- config.setAllowCredentials(true);
+ // Fail closed: register no CORS rule → Spring rejects cross-origin requests to auth paths.
+ log.warn("plate.auth.cors.allowed-origins is empty — CORS disabled for plate-auth endpoints "
+ + "(same-origin only). Set allowed-origins to enable cross-origin access.");
+ 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.setAllowedHeaders(List.of("*"));
config.setMaxAge(3600L);
- UrlBasedCorsConfigurationSource source = new UrlBasedCorsConfigurationSource();
source.registerCorsConfiguration("/**", config);
return source;
}
diff --git a/plate-auth-starter/src/main/java/de/platesoft/auth/entity/RefreshToken.java b/plate-auth-starter/src/main/java/de/platesoft/auth/entity/RefreshToken.java
deleted file mode 100644
index f908fac..0000000
--- a/plate-auth-starter/src/main/java/de/platesoft/auth/entity/RefreshToken.java
+++ /dev/null
@@ -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();
- }
-}
diff --git a/plate-auth-starter/src/main/java/de/platesoft/auth/repository/RefreshTokenRepository.java b/plate-auth-starter/src/main/java/de/platesoft/auth/repository/RefreshTokenRepository.java
deleted file mode 100644
index a526b77..0000000
--- a/plate-auth-starter/src/main/java/de/platesoft/auth/repository/RefreshTokenRepository.java
+++ /dev/null
@@ -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 Transaction contract (Review-v2 W-A). Both methods are invoked inside 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:
+ *
+ *
+ * Future v0.2 may move these calls to a post-commit phase; until then assume "in-transaction".
*/
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);
default void onSubsequentSignIn(User user, LoginProvider provider) {