Skip to content
Merged
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
7 changes: 4 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ Security hardening from the SUF review series (SUF-01 through SUF-06). Most chan
### Security
- **SUF-01 — Host-header link poisoning (CWE-640): the ordinary request host is now allow-listed, not just `X-Forwarded-Host`.** When `user.security.trustedHosts` is configured, the finally-chosen host for a password-reset / verification link — including the ordinary request server name, which on common servlet containers is derived from the attacker-influenceable `Host` header — must be in the allow-list. A non-allow-listed host now falls back to the first configured trusted host (a known-good canonical authority) instead of being emitted into the link. Matching on this ordinary-host path is case-insensitive (RFC 4343; previously only the forwarded-host path was), and blank/whitespace `trustedHosts` entries are ignored.
- New opt-in property **`user.security.requireCanonicalAppUrl`** (default `false`): when `true`, application startup fails unless `user.security.appUrl` or a non-empty `user.security.trustedHosts` is configured — a hard guarantee that email links can never derive their authority from a spoofable `Host` header. The default stays `false` in this release (a startup warning is logged instead), and is **planned to become the default in a future major version**. Setting `user.security.appUrl` (recommended) or `trustedHosts` in production is advised regardless of this flag.
- **SUF-02 — Step-up required to set an initial password on a passwordless account.** `POST /user/setPassword` adds a durable password to a passwordless (passkey-only) account, but previously required only an authenticated session. A new SPI, **`StepUpService`**, lets a consuming application require a fresh proof of presence/possession (WebAuthn assertion, TOTP, recent-auth) before the operation. If a `StepUpService` bean is present it is required (failure → `HTTP 401`); if none is present the endpoint is **disabled by default** (`HTTP 403`). Set **`user.security.allowInitialPasswordSetWithoutStepUp=true`** to keep the previous session-only behavior. (The full WebAuthn step-up primitive is tracked separately; this ships the interim guard.)
- **SUF-02 — Step-up required to set an initial password on a passwordless account.** `POST /user/setPassword` adds a durable password to a passwordless (passkey-only) account, but previously required only an authenticated session. A new SPI, **`StepUpService`**, lets a consuming application require a fresh proof of presence/possession (WebAuthn assertion, TOTP, recent-auth) before the operation. If a `StepUpService` bean is present it is required (failure → `HTTP 401`); if none is present the endpoint is **disabled by default** (`HTTP 403`). Set **`user.security.allowInitialPasswordSetWithoutStepUp=true`** to keep the previous session-only behavior. The two failure modes use distinct response `code` values so a client can tell them apart — step-up denied is `code 6` (`HTTP 401`), disabled-by-default is `code 7` (`HTTP 403`) — and the library logs a startup warning when the endpoint is left disabled-by-default so the state is visible at boot rather than only when a user hits the `403`. (The full WebAuthn step-up primitive is tracked separately; this ships the interim guard.)
- **SUF-03 — Revoke every user session on account delete/disable, after the change commits.** Deleting or disabling an account now revokes *all* of that user's active sessions (not just the caller's current request; `DSUserDetails` caches the enabled flag at login and is not re-checked per request). Revocation is deferred until *after* the delete/disable transaction commits, closing a race where a login landing between the session scan and the commit could register a new, surviving session. If the transaction rolls back, sessions are no longer revoked.
- **SUF-04 — Authenticated password change participates in brute-force lockout.** `POST /user/updatePassword` now rejects a locked account up front with `HTTP 423 Locked`, reports a wrong current password to `LoginAttemptService` (counting toward lockout, matching the login path), and resets the counter on success. **Passwordless (passkey-only / OAuth-only) accounts** — which have no current password to verify — are rejected up front with `HTTP 400` (the message directs the user to `POST /user/setPassword`) and **never feed the lockout counter**, preventing a session-holding caller from locking such an account out of every authentication method.
- **SUF-05 — Password-reset token no longer leaks via Referer or caches (CWE-598).** The reset flow carries the token in the page URL; the library now sets `Referrer-Policy: no-referrer` and `Cache-Control: no-store` on the reset pages (via a scoped interceptor over the configured reset URIs) so the token is not sent in the `Referer` header or written to shared/browser caches. The token-in-body `POST /user/savePassword` contract is unchanged (non-breaking); residual address-bar/history exposure remains and is documented.
Expand All @@ -19,15 +19,16 @@ Security hardening from the SUF review series (SUF-01 through SUF-06). Most chan
- **SUF-06 (code hardening)** — `showChangePasswordPage` no longer mints an `HttpSession` for the anonymous token-validation request (session amplification), and audits an invalid token as `Failure` rather than `Success`. Audit-log rotation intentionally remains disabled by default (rotated archives are excluded from GDPR/audit queries); rate-limiting anonymous endpoints remains delegated to Bucket4J.

### Behavior changes (client impact)
- **`POST /user/setPassword` is disabled by default (SUF-02).** It returns `HTTP 403` unless a `StepUpService` bean is provided (then step-up is required; failures return `HTTP 401`), or `user.security.allowInitialPasswordSetWithoutStepUp=true` is set to restore the previous session-only behavior. Consumers whose passwordless users set an initial password must choose one of those options.
- **`POST /user/setPassword` is disabled by default (SUF-02).** It returns `HTTP 403` (response `code 7`) unless a `StepUpService` bean is provided (then step-up is required; failures return `HTTP 401`, response `code 6`), or `user.security.allowInitialPasswordSetWithoutStepUp=true` is set to restore the previous session-only behavior. Consumers whose passwordless users set an initial password must choose one of those options.
- `POST /user/updatePassword` can now return `HTTP 423 Locked` (account locked) in addition to its existing `200`/`400`. Clients that treat any non-`200` as a generic failure need no change; clients that branch on status can surface the locked state. A passwordless account calling this endpoint receives `400` with a "no password set" message and is directed to `POST /user/setPassword`.

### Documentation
- `CONFIG.md`: documented `user.security.requireCanonicalAppUrl`, the ordinary-host allow-list behavior, and `user.security.allowInitialPasswordSetWithoutStepUp` / the `StepUpService` SPI; corrected the account-lockout property prefixes to `user.security.*`; noted that `/user/updatePassword` participates in lockout.
- `MIGRATION.md`: the credential-changes section no longer states `/user/updatePassword` is "unchanged"; documented the `setPassword` step-up requirement / default-disabled behavior and the opt-in flag; documented the `requireCanonicalAppUrl` opt-in and corrected the trusted-host fallback description.
- Registered `user.security.requireCanonicalAppUrl` and `user.security.allowInitialPasswordSetWithoutStepUp` in the Spring configuration metadata (`additional-spring-configuration-metadata.json`) so both surface in IDE property auto-completion. Clarified the `StepUpService` SPI Javadoc: the request body has already been consumed by the target endpoint's `@RequestBody` binding before `isStepUpSatisfied` runs, so implementations must read the step-up proof from a header, request parameter, or the session (not the body).

### Testing
- Added unit and full-context coverage: passwordless `updatePassword` rejection without touching the lockout counter; end-to-end lockout through a real Spring context; session revocation deferred to after commit (delete and disable paths); `AppUrlResolver` ordinary-host allow-list (blank-entry filtering, case-insensitive matching, IPv6 literal); `user.security.requireCanonicalAppUrl` through real `@Value` binding plus strict-mode assertions; `setPassword` step-up guard (disabled-by-default, opt-in flag, `StepUpService` grant/deny); reset-page security headers; and the anonymous-session / audit-status hygiene on `showChangePasswordPage`.
- Added unit and full-context coverage: passwordless `updatePassword` rejection without touching the lockout counter; end-to-end lockout through a real Spring context; session revocation deferred to after commit (delete and disable paths); `AppUrlResolver` ordinary-host allow-list (blank-entry filtering, case-insensitive matching, IPv6 literal); `user.security.requireCanonicalAppUrl` through real `@Value` binding plus strict-mode assertions; `setPassword` step-up guard (disabled-by-default, opt-in flag, `StepUpService` grant/deny); the distinct disabled-vs-denied response codes; the startup warning emitted when `setPassword` is left disabled-by-default; reset-page security headers; and the anonymous-session / audit-status hygiene on `showChangePasswordPage`. Added full-context tests that boot the real `ObjectProvider<StepUpService>` (a passwordless `setPassword` returns `403`) and verify the `WebInterceptorConfig`-wired reset-page headers survive the Spring Security filter chain.

## [5.0.1] - 2026-06-15
### Features
Expand Down
24 changes: 23 additions & 1 deletion src/main/java/com/digitalsanctuary/spring/user/api/UserAPI.java
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import com.digitalsanctuary.spring.user.util.AppUrlResolver;
import com.digitalsanctuary.spring.user.util.JSONResponse;
import com.digitalsanctuary.spring.user.util.UserUtils;
import jakarta.annotation.PostConstruct;
import jakarta.servlet.ServletException;
import jakarta.servlet.http.HttpServletRequest;
import lombok.RequiredArgsConstructor;
Expand Down Expand Up @@ -113,6 +114,25 @@ public class UserAPI {
@Value("${user.security.allowInitialPasswordSetWithoutStepUp:false}")
private boolean allowInitialPasswordSetWithoutStepUp;

/**
* SUF-02: warn at startup when {@code POST /user/setPassword} is disabled by default &mdash; i.e. no
* {@link StepUpService} bean is present and {@code user.security.allowInitialPasswordSetWithoutStepUp} is left
* {@code false}. In that state a passwordless (passkey-only) account cannot set an initial password and every request
* returns HTTP 403. Surfacing this at boot (mirroring the SUF-01 fail-fast/warn pattern in
* {@code UserSecurityBeansAutoConfiguration}) means operators learn about the disabled state at startup instead of
* discovering it only when a user hits the 403 in production.
*/
@PostConstruct
void warnIfInitialPasswordSetDisabled() {
if (stepUpServiceProvider.getIfAvailable() == null && !allowInitialPasswordSetWithoutStepUp) {
log.warn("UserAPI: POST /user/setPassword is disabled by default - no StepUpService bean is configured and "
+ "user.security.allowInitialPasswordSetWithoutStepUp is false, so passwordless (passkey-only) accounts "
+ "cannot set an initial password (every request returns HTTP 403). Provide a StepUpService bean to "
+ "require step-up verification (recommended), or set user.security.allowInitialPasswordSetWithoutStepUp=true "
+ "to explicitly allow the session-only behavior. (SUF-02)");
}
}

/**
* Registers a new user account.
*
Expand Down Expand Up @@ -573,8 +593,10 @@ public ResponseEntity<JSONResponse> setPassword(@AuthenticationPrincipal DSUserD
}
} else if (!allowInitialPasswordSetWithoutStepUp) {
logAuditEvent("SetPassword", "Failure", "Initial password set disabled (no step-up configured)", user, request);
// Distinct code (7) from the step-up-denied branch (6) so callers can tell "disabled on this server" (403)
// apart from "step-up verification failed" (401).
return buildErrorResponse(messages.getMessage("message.set-password.disabled", null,
"Setting an initial password is not enabled on this server.", locale), 6, HttpStatus.FORBIDDEN);
"Setting an initial password is not enabled on this server.", locale), 7, HttpStatus.FORBIDDEN);
}

if (!setPasswordDto.getNewPassword().equals(setPasswordDto.getConfirmPassword())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,12 @@ public interface StepUpService {
* @param user the authenticated user the operation targets (never {@code null})
* @param action a short, stable action identifier (e.g. {@code "set-password"})
* @param request the current HTTP request, so the implementation can read a step-up assertion/token supplied by the
* client (headers, body, or a prior challenge stored in the session)
* client. Read it from request headers, query/form parameters, or a prior challenge stored in the session.
* Note: by the time this method runs the request <em>body</em> has already been consumed by the target
* endpoint's {@code @RequestBody} binding (e.g. {@code SetPasswordDto} on {@code POST /user/setPassword}), so
* {@code request.getInputStream()}/{@code request.getReader()} will be empty. Carry the proof in a header, a request
* parameter, or the session instead &mdash; or install a content-caching request filter if you must re-read
* the body.
* @return {@code true} if step-up is satisfied and the operation may proceed; {@code false} to reject it
*/
boolean isStepUpSatisfied(User user, String action, HttpServletRequest request);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,18 @@
"type": "java.util.List<java.lang.String>",
"description": "When user.security.appUrl is not set, X-Forwarded-Host is honored only for hosts in this comma-separated allowlist; otherwise the container's own server name is used."
},
{
"name": "user.security.requireCanonicalAppUrl",
"type": "java.lang.Boolean",
"description": "When true, fail startup unless user.security.appUrl or a non-empty user.security.trustedHosts is configured, so security email links (password reset, verification) can never derive their authority from a spoofable Host header (CWE-640). When false (default), the library logs a startup warning instead of failing.",
"defaultValue": false
},
{
"name": "user.security.allowInitialPasswordSetWithoutStepUp",
"type": "java.lang.Boolean",
"description": "Controls the fallback behavior of POST /user/setPassword when no StepUpService bean is present. When false (default), setting an initial password on a passwordless (passkey-only) account is disabled (HTTP 403) unless a StepUpService is provided; set to true to explicitly allow the session-only behavior (SUF-02).",
"defaultValue": false
},
{
"name": "user.security.loginActionURI",
"type": "java.lang.String",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;

import ch.qos.logback.classic.Level;
import ch.qos.logback.classic.Logger;
import ch.qos.logback.classic.spi.ILoggingEvent;
import ch.qos.logback.core.read.ListAppender;
import java.util.Collections;
import java.util.Locale;

Expand Down Expand Up @@ -50,6 +54,7 @@
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.slf4j.LoggerFactory;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.context.MessageSource;
import org.springframework.http.HttpStatus;
Expand Down Expand Up @@ -690,7 +695,9 @@ void setPassword_noStepUpService_disabledByDefault() throws Exception {
.content(objectMapper.writeValueAsString(newSetPasswordDto()))
.with(csrf()))
.andExpect(status().isForbidden())
.andExpect(jsonPath("$.success").value(false));
.andExpect(jsonPath("$.success").value(false))
// Distinct code from the step-up-denied (401) branch so a client can disambiguate "disabled" from "denied".
.andExpect(jsonPath("$.code").value(7));

// Endpoint disabled by default: no step-up service, opt-in flag off -> the credential is never set.
verify(userService, never()).setInitialPassword(any(), any());
Expand Down Expand Up @@ -734,7 +741,9 @@ void setPassword_stepUpService_deniesReturns401() throws Exception {
.content(objectMapper.writeValueAsString(newSetPasswordDto()))
.with(csrf()))
.andExpect(status().isUnauthorized())
.andExpect(jsonPath("$.success").value(false));
.andExpect(jsonPath("$.success").value(false))
// Distinct code from the disabled-by-default (403) branch.
.andExpect(jsonPath("$.code").value(6));

verify(userService, never()).setInitialPassword(any(), any());
}
Expand Down Expand Up @@ -762,6 +771,74 @@ void setPassword_stepUpService_grantsProceeds() throws Exception {
}
}

@Nested
@DisplayName("SUF-02: setPassword disabled-by-default startup warning")
class SetPasswordStartupWarning {

private ListAppender<ILoggingEvent> appender;
private Logger apiLogger;

@SuppressWarnings("unchecked")
private ObjectProvider<StepUpService> providerOf(StepUpService service) {
ObjectProvider<StepUpService> provider = mock(ObjectProvider.class);
when(provider.getIfAvailable()).thenReturn(service);
return provider;
}

@BeforeEach
void attachAppender() {
apiLogger = (Logger) LoggerFactory.getLogger(UserAPI.class);
appender = new ListAppender<>();
appender.start();
apiLogger.addAppender(appender);
}

@AfterEach
void detachAppender() {
apiLogger.detachAppender(appender);
}

private long disabledWarnings() {
return appender.list.stream()
.filter(event -> event.getLevel() == Level.WARN)
.filter(event -> event.getFormattedMessage().contains("/user/setPassword is disabled by default"))
.count();
}

@Test
@DisplayName("warns at startup when no StepUpService bean and the opt-in flag is false")
void warnsWhenDisabledByDefault() {
ReflectionTestUtils.setField(userAPI, "stepUpServiceProvider", providerOf(null));
ReflectionTestUtils.setField(userAPI, "allowInitialPasswordSetWithoutStepUp", false);

userAPI.warnIfInitialPasswordSetDisabled();

assertThat(disabledWarnings()).isEqualTo(1);
}

@Test
@DisplayName("does not warn when a StepUpService bean is present")
void doesNotWarnWhenStepUpServicePresent() {
ReflectionTestUtils.setField(userAPI, "stepUpServiceProvider", providerOf(mock(StepUpService.class)));
ReflectionTestUtils.setField(userAPI, "allowInitialPasswordSetWithoutStepUp", false);

userAPI.warnIfInitialPasswordSetDisabled();

assertThat(disabledWarnings()).isZero();
}

@Test
@DisplayName("does not warn when the opt-in flag is enabled")
void doesNotWarnWhenFlagEnabled() {
ReflectionTestUtils.setField(userAPI, "stepUpServiceProvider", providerOf(null));
ReflectionTestUtils.setField(userAPI, "allowInitialPasswordSetWithoutStepUp", true);

userAPI.warnIfInitialPasswordSetDisabled();

assertThat(disabledWarnings()).isZero();
}
}

@Nested
@DisplayName("User Profile Tests")
class UserProfileTests {
Expand Down
Loading
Loading