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
14 changes: 9 additions & 5 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,30 @@ All notable changes to this project are documented here. This project follows [S

## [Unreleased]

Security hardening from the SUF review series (SUF-01, SUF-03, SUF-04) plus a documentation correction (SUF-06). All changes are backward compatible; the one new property is opt-in and defaults to the pre-existing behavior.
Security hardening from the SUF review series (SUF-01 through SUF-06). Most changes are backward compatible. The one notable behavior change: `POST /user/setPassword` is now **disabled by default** (SUF-02) — see **Behavior changes** below.

### 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-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.

### Fixes
- **SUF-06** — Corrected the documented audit-log rotation default in `CONFIG.md`.
- **SUF-06 (docs)** — Corrected the documented audit-log rotation default in `CONFIG.md` (rotation stays opt-in / `0` by design).
- **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/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` and the ordinary-host allow-list behavior; 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" (it now enforces lockout and rejects passwordless accounts); documented the `requireCanonicalAppUrl` opt-in and corrected the trusted-host fallback description.
- `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.

### Testing
- Added unit and full-context integration 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); and `user.security.requireCanonicalAppUrl` through real `@Value` binding (`CoreBeanOverrideTest`) plus strict-mode tests that assert the resolver actually uses the configured values.
- 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`.

## [5.0.1] - 2026-06-15
### Features
Expand Down
7 changes: 7 additions & 0 deletions CONFIG.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,13 @@ Password-reset and verification emails contain a link back to your application.

When neither `appUrl` nor `trustedHosts` is set, links are built from the request host (backward-compatible behavior) and a startup warning is logged.

### Passwordless Initial Password Step-Up (SUF-02)

`POST /user/setPassword` adds an *initial* password to a passwordless (passkey-only) account. Because there is no current credential to verify, the endpoint is gated:

- **Step-Up Service (`com.digitalsanctuary.spring.user.security.StepUpService` SPI)**: If your application provides a `StepUpService` bean, it is **required** — `setPassword` proceeds only when `isStepUpSatisfied(user, "set-password", request)` returns `true`; otherwise it returns `HTTP 401`. Implement it to require a fresh WebAuthn assertion, TOTP, or recent-auth proof.
- **Allow Without Step-Up (`user.security.allowInitialPasswordSetWithoutStepUp`)**: When no `StepUpService` bean is present, `setPassword` is **disabled** (`HTTP 403`) unless this is `true`, which restores the previous session-only behavior. Default: `false`.

### Token Security

Verification and password-reset tokens are **hashed at rest**. The raw token is only ever sent to the user in the emailed link; the database stores its hash. Lookups hash the incoming token and match by hash, with a transparent fallback to plaintext lookup so that any links issued before upgrading keep working until they expire. This requires no schema migration and no action from consuming applications.
Expand Down
11 changes: 9 additions & 2 deletions MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -150,9 +150,16 @@ Affected endpoints (all require `user.webauthn.enabled=true` except where noted)

**Passwordless (passkey-only) accounts — residual risk:** For accounts with no password set, there is no current credential to verify, and this library does not yet implement a WebAuthn step-up assertion (a feasible recent-authentication signal does not currently exist in the framework). As a result:
- Deleting or renaming a passkey on a passwordless account remains a session-only operation (last-credential lockout protection and ownership checks still apply).
- Setting an *initial* password via `POST /user/setPassword` on a passwordless account also cannot require a current password (there is none); this endpoint still rejects accounts that already have a password.
- Setting an *initial* password via `POST /user/setPassword` cannot require a current password (there is none). **As of the SUF-02 hardening this endpoint is disabled by default** — it returns `HTTP 403` unless you either provide a step-up service (below) or explicitly opt into the previous behavior. It still rejects accounts that already have a password.

This is a deliberate, documented limitation rather than a half-measure: implementing a true WebAuthn step-up assertion would require significant new challenge/response infrastructure. Consuming applications that need stronger guarantees for passwordless accounts can front these endpoints with their own step-up (e.g. require a fresh passkey assertion) before allowing the call. This will be revisited if/when a step-up mechanism is added to the framework.
**Step-up SPI and the `setPassword` default (SUF-02).** A new SPI, `com.digitalsanctuary.spring.user.security.StepUpService`, lets you require a fresh proof of presence/possession (WebAuthn assertion, TOTP, recent-auth) before `setPassword` proceeds:

- If you register a `StepUpService` bean, it is **required**: `setPassword` proceeds only when `isStepUpSatisfied(...)` returns `true`, otherwise it returns `HTTP 401`.
- If no `StepUpService` bean is present, `setPassword` is **disabled** (`HTTP 403`) unless you set `user.security.allowInitialPasswordSetWithoutStepUp=true`, which restores the prior session-only behavior.

**Action required if your application lets passwordless users set an initial password:** provide a `StepUpService` bean (recommended), or set `user.security.allowInitialPasswordSetWithoutStepUp=true`. Otherwise `POST /user/setPassword` returns `403`.

Implementing a *full* WebAuthn step-up assertion (challenge/response bound to user/session/action/RP ID/origin/expiry) is tracked as separate feature work; the `StepUpService` SPI is the interim mechanism so applications can enforce their own step-up now.

### Database schema: unique role/privilege names

Expand Down
38 changes: 33 additions & 5 deletions src/main/java/com/digitalsanctuary/spring/user/api/UserAPI.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import com.digitalsanctuary.spring.user.persistence.model.User;
import com.digitalsanctuary.spring.user.registration.RegistrationDeniedException;
import com.digitalsanctuary.spring.user.registration.RegistrationGuard;
import com.digitalsanctuary.spring.user.security.StepUpService;
import com.digitalsanctuary.spring.user.service.DSUserDetails;
import com.digitalsanctuary.spring.user.service.LoginAttemptService;
import com.digitalsanctuary.spring.user.service.PasswordPolicyService;
Expand Down Expand Up @@ -92,6 +93,8 @@ public class UserAPI {
private final ObjectProvider<WebAuthnCredentialManagementService> webAuthnCredentialManagementServiceProvider;
private final AppUrlResolver appUrlResolver;
private final LoginAttemptService loginAttemptService;
/** Optional consumer-provided step-up (re-)authentication service; see {@link StepUpService} (SUF-02). */
private final ObjectProvider<StepUpService> stepUpServiceProvider;

@Value("${user.security.registrationPendingURI}")
private String registrationPendingURI;
Expand All @@ -102,6 +105,14 @@ public class UserAPI {
@Value("${user.security.forgotPasswordPendingURI}")
private String forgotPasswordPendingURI;

/**
* SUF-02: controls the fallback behavior of {@code /user/setPassword} when no {@link StepUpService} bean is present.
* When {@code false} (the default), setting an initial password on a passwordless account is disabled unless a
* {@link StepUpService} is provided; set to {@code true} to explicitly allow the session-only behavior.
*/
@Value("${user.security.allowInitialPasswordSetWithoutStepUp:false}")
private boolean allowInitialPasswordSetWithoutStepUp;

/**
* Registers a new user account.
*
Expand Down Expand Up @@ -519,14 +530,14 @@ public ResponseEntity<JSONResponse> registerPasswordlessAccount(@Valid @RequestB
* This endpoint only applies to passwordless (passkey-only) accounts and rejects the request if a password is already
* set (use {@code /user/updatePassword} to change an existing password, which requires the current password). Because
* the account has no current password to verify, this credential-altering operation cannot require re-authentication
* via a current password, and this library does not yet implement a WebAuthn step-up assertion.
* via a current password.
* </p>
*
* <p>
* <strong>Residual risk:</strong> a session-only actor on a passwordless account could set an initial password. This is
* a known, documented limitation (see MIGRATION.md "Re-authentication required for credential changes"). It is not a
* regression and is bounded: the new password does not displace any existing credential, and consuming applications can
* front this endpoint with their own step-up if required.
* <strong>Step-up (SUF-02):</strong> to address the residual risk that a session-only actor could add a durable
* password, this endpoint requires step-up when a {@link StepUpService} bean is provided, and is otherwise
* <strong>disabled by default</strong>. Set {@code user.security.allowInitialPasswordSetWithoutStepUp=true} to keep
* the previous session-only behavior when no step-up service is available. See MIGRATION.md.
* </p>
*
* @param userDetails the authenticated user details
Expand All @@ -549,6 +560,23 @@ public ResponseEntity<JSONResponse> setPassword(@AuthenticationPrincipal DSUserD
return buildErrorResponse("User already has a password. Use the change password feature instead.", 1, HttpStatus.BAD_REQUEST);
}

// SUF-02: setting an initial password on a passwordless (passkey-only) account is a credential change with no
// current credential to verify, so a session-only actor could otherwise add a durable password. If a consumer
// supplies a StepUpService, require it to pass; otherwise the endpoint is disabled by default. Set
// user.security.allowInitialPasswordSetWithoutStepUp=true to explicitly keep the session-only behavior.
final StepUpService stepUpService = stepUpServiceProvider.getIfAvailable();
if (stepUpService != null) {
if (!stepUpService.isStepUpSatisfied(user, "set-password", request)) {
logAuditEvent("SetPassword", "Failure", "Step-up verification failed", user, request);
return buildErrorResponse(messages.getMessage("message.set-password.step-up-required", null,
"Additional verification is required to set a password.", locale), 6, HttpStatus.UNAUTHORIZED);
}
} else if (!allowInitialPasswordSetWithoutStepUp) {
logAuditEvent("SetPassword", "Failure", "Initial password set disabled (no step-up configured)", user, request);
return buildErrorResponse(messages.getMessage("message.set-password.disabled", null,
"Setting an initial password is not enabled on this server.", locale), 6, HttpStatus.FORBIDDEN);
}

if (!setPasswordDto.getNewPassword().equals(setPasswordDto.getConfirmPassword())) {
return buildErrorResponse(messages.getMessage("message.password.mismatch", null, "Passwords do not match", locale), 2,
HttpStatus.BAD_REQUEST);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import com.digitalsanctuary.spring.user.util.UserUtils;
import com.digitalsanctuary.spring.user.web.IncludeUserInModel;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpSession;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;

Expand Down Expand Up @@ -82,13 +83,20 @@ public ModelAndView showChangePasswordPage(final HttpServletRequest request, fin
log.debug("UserAPI.showChangePasswordPage: called with token: {}", TokenHasher.fingerprint(token));
final TokenValidationResult result = userService.validatePasswordResetToken(token);
log.debug("UserAPI.showChangePasswordPage: result: {}", result);
AuditEvent changePasswordAuditEvent = AuditEvent.builder().source(this).sessionId(request.getSession().getId())
final boolean valid = TokenValidationResult.VALID.equals(result);
// SUF-06: this is an anonymous, token-validating GET. Do NOT mint an HttpSession just to audit it (a stream of
// anonymous invalid-token probes would otherwise accumulate server-side session state); reuse an existing
// session id only if the caller already has one.
final HttpSession existingSession = request.getSession(false);
final String sessionId = existingSession != null ? existingSession.getId() : null;
AuditEvent changePasswordAuditEvent = AuditEvent.builder().source(this).sessionId(sessionId)
.ipAddress(UserUtils.getClientIP(request)).userAgent(request.getHeader("User-Agent"))
.action("showChangePasswordPage")
.actionStatus("Success").message("Requested. Result:" + result).build();
// SUF-06: label by validity so anonymous invalid-token attempts are not recorded as "Success".
.actionStatus(valid ? "Success" : "Failure").message("Requested. Result:" + result).build();

eventPublisher.publishEvent(changePasswordAuditEvent);
if (TokenValidationResult.VALID.equals(result)) {
if (valid) {
model.addAttribute("token", token);
String redirectString = "redirect:" + forgotPasswordChangeURI;
return new ModelAndView(redirectString, model);
Expand Down
Loading
Loading