feat: built-in WebAuthn step-up for credential-altering operations - #365
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an opt-in, built-in StepUpService implementation based on Spring Security factor freshness (defaulting to WebAuthn) to protect credential-altering operations in this library, while preserving the existing “off by default” contract unless user.security.stepUp.enabled=true.
Changes:
- Introduces
StepUpConfigProperties,StepUpAutoConfiguration, andDSFactorFreshnessStepUpServiceto provide a default step-up mechanism when enabled. - Extends step-up gating to WebAuthn credential rename/delete for passwordless accounts (401 +
step-up-required), and enables factor-merging when step-up is enabled (even if MFA is off). - Adds a startup validator to reject
FACTOR_*role/privilege names that would collide with Spring Security factor authorities; updates docs and adds tests.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/java/com/digitalsanctuary/spring/user/security/StepUpIntegrationTest.java | Full-context test that step-up registers the service and enables filter merging with MFA off |
| src/test/java/com/digitalsanctuary/spring/user/security/StepUpAutoConfigurationTest.java | Auto-config contract tests: default-off, enablement, backoff, factor validation, validator presence |
| src/test/java/com/digitalsanctuary/spring/user/security/MfaFilterMergingConfigurationTest.java | Ensures merging post-processor is registered when step-up alone is enabled |
| src/test/java/com/digitalsanctuary/spring/user/security/FactorAuthorityNameValidatorTest.java | Tests for reserved FACTOR_* authority-name detection and fail/log behavior |
| src/test/java/com/digitalsanctuary/spring/user/security/DSFactorFreshnessStepUpServiceTest.java | Unit tests for factor freshness and caller identity checks |
| src/test/java/com/digitalsanctuary/spring/user/api/WebAuthnManagementAPITest.java | Updates existing API tests for new method signatures and request parameter |
| src/test/java/com/digitalsanctuary/spring/user/api/WebAuthnManagementAPIStepUpTest.java | New tests for step-up enforcement on passkey rename/delete |
| src/main/resources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports | Registers StepUpAutoConfiguration in auto-config imports |
| src/main/resources/config/dsspringuserconfig.properties | Documents new user.security.stepUp.* properties and defaults |
| src/main/java/com/digitalsanctuary/spring/user/security/StepUpConfigProperties.java | New configuration properties for step-up enablement/TTL/factors |
| src/main/java/com/digitalsanctuary/spring/user/security/StepUpAutoConfiguration.java | Auto-config that validates factors, wires default StepUpService, and registers authority-name validator |
| src/main/java/com/digitalsanctuary/spring/user/security/MfaFilterMergingConfiguration.java | Enables factor merging when MFA or step-up is enabled |
| src/main/java/com/digitalsanctuary/spring/user/security/FactorAuthorityNameValidator.java | New startup validator preventing FACTOR_* authority collisions |
| src/main/java/com/digitalsanctuary/spring/user/security/DSFactorFreshnessStepUpService.java | Default step-up implementation using factor issued-at freshness |
| src/main/java/com/digitalsanctuary/spring/user/exceptions/WebAuthnStepUpRequiredException.java | New exception for “step-up required” signaling (401 + distinct error code) |
| src/main/java/com/digitalsanctuary/spring/user/api/WebAuthnManagementAPIAdvice.java | Maps step-up-required exception to 401 with step-up-required code |
| src/main/java/com/digitalsanctuary/spring/user/api/WebAuthnManagementAPI.java | Adds step-up gating for passkey rename/delete on passwordless accounts and threads HttpServletRequest through |
| src/main/java/com/digitalsanctuary/spring/user/api/UserAPI.java | Updates references in comments to renamed gating method |
| MIGRATION.md | Documents built-in step-up behavior, reserved name validation, and factor-merging implications |
| CONFIG.md | Adds full step-up configuration + client contract documentation |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| * {@link #requireCredentialProof} and result in HTTP 400 with the message | ||
| * {@code "Current password is required to change authentication methods."}. A blank or incorrect | ||
| * {@code currentPassword} is likewise rejected with a 400 before any mutation occurs. |
| Set<String> offenders = new TreeSet<>(); | ||
| rolesAndPrivilegesConfig.getRolesAndPrivileges().forEach((role, privileges) -> Stream | ||
| .concat(Stream.of(role), privileges == null ? Stream.<String>empty() : privileges.stream()) | ||
| .filter(name -> name != null && name.toUpperCase().startsWith(FACTOR_PREFIX)).forEach(offenders::add)); |
| @Data | ||
| @ConfigurationProperties(prefix = "user.security.step-up") | ||
| public class StepUpConfigProperties { |
| public void setSecurityContextHolderStrategy(SecurityContextHolderStrategy securityContextHolderStrategy) { | ||
| this.securityContextHolderStrategy = securityContextHolderStrategy; | ||
| } |
| Set<String> unknown = factors.stream().filter(factor -> factor == null || factor.isBlank() | ||
| || !StepUpConfigProperties.FACTOR_AUTHORITIES.containsKey(factor.toUpperCase())) | ||
| .map(factor -> factor == null ? "null" : factor).collect(Collectors.toCollection(TreeSet::new)); |
|
Review: feat: built-in WebAuthn step-up for credential-altering operations Nicely scoped feature: off by default, reuses the existing login ceremony instead of inventing a new challenge protocol, and the FactorAuthorityNameValidator fix is a genuine, well-reasoned bypass closure that is worth its own praise (fail-fast when MFA/step-up are enabled, fail-loud otherwise). The test suite is thorough (freshness, TTL boundaries, caller-identity checks, factor selection, AnyNestedCondition wiring, full-context integration). Two things worth a look before merge: 1. Stale Javadoc in WebAuthnManagementAPI (correctness of docs) src/main/java/com/digitalsanctuary/spring/user/api/WebAuthnManagementAPI.java (class-level Javadoc, "Residual risk" paragraph, around L65-68) still reads:
That is exactly what this PR fixes: requireCredentialProof now gates passwordless delete/rename through StepUpService when one is configured. The method-level Javadoc on requireCredentialProof was correctly updated, but this class-level paragraph was missed and now contradicts the actual behavior (and the CONFIG.md/MIGRATION.md updates in the same PR). Worth a follow-up edit so generated docs do not mislead consumers about the residual risk. 2. StepUpConfigProperties.ttlSeconds @min(1) is inert without @validated src/main/java/com/digitalsanctuary/spring/user/security/StepUpConfigProperties.java adds @min(1) on ttlSeconds, but the class itself is not annotated @validated. Per the Spring Boot @ConfigurationProperties validation model, JSR-303 constraints are only enforced when the properties class carries @validated, and that is exactly the pattern this repo already follows elsewhere (UserSecurityConfigProperties, PasswordPolicyConfigProperties both pair their @Min/etc annotations with @validated). As written, a misconfigured user.security.stepUp.ttlSeconds=0 (or negative) will not fail fast at startup the way StepUpAutoConfiguration.validateFactors deliberately does for factor names; it will silently bind and only surface as strange runtime behavior in Duration.ofSeconds(...)/RequiredFactor.validDuration(...). Given how much of this PR is about failing startup rather than producing a confusing runtime symptom, this one slipped through. Suggest adding @validated to the class. Other notes (non-blocking)
Nothing here blocks merge on its own; both flagged items are documentation/validation-robustness gaps rather than functional bugs, but worth fixing given this PR own stated bar of failing startup rather than producing an operation nobody can perform. |
Ships a default StepUpService so consumers get step-up out of the box instead of implementing the SPI themselves (#335). Off by default. Step-up is expressed as a freshness requirement on a Spring Security factor: WebAuthnAuthenticationProvider stamps FACTOR_WEBAUTHN with the moment of assertion, and re-running the ordinary passkey ceremony while already logged in merges a newly stamped factor onto the session. So the ceremony is the existing login flow, re-run. No new endpoints, no server-side challenge state, no step-up token, and no second client ceremony to write. - user.security.stepUp.{enabled,ttlSeconds,factors}, defaults false/120/ [WEBAUTHN]. Unknown or empty factor names fail startup. - DSFactorFreshnessStepUpService satisfies the SPI shipped in #334 unchanged, so consumer implementations keep working and still win via @ConditionalOnMissingBean. - Passkey delete/rename now go through the same gate. They previously required nothing at all on a passwordless account: the current-password check returns immediately when there is no password. Enforcement only applies when a StepUpService is configured, so existing deployments are unaffected. Rejections are 401 with error code step-up-required. - Enabling step-up now also enables factor merging, which was tied to user.mfa.enabled. Without it a re-assertion replaces the session's authorities instead of refreshing the factor on them. Also adds FactorAuthorityNameValidator, which rejects FACTOR_-prefixed role and privilege names. Such a name is indistinguishable from a real factor by name, and the authorities loaded from the database sort ahead of the stamped one: it satisfies MFA enforcement without the factor ever being completed (an authentication bypass for that deployment), and it shadows the genuine factor in a step-up freshness check. Startup fails when MFA or step-up is enabled, and logs an error otherwise. Binding model, per the decisions on #335: the proof is bound to user, session, and time, but is not single-use and not per-action. That closes the case the feature exists for, an attacker with a session cookie and no authenticator. It leaves an attacker sharing the session able to piggyback inside the window, hence the short default TTL. Docs: CONFIG.md and MIGRATION.md. ./gradlew check: 1204 tests, 0 failures. Refs #335
9660d18 to
103e8fe
Compare
Status: parked pending a design decisionRebased onto 5.3.2. Recording the review findings and why this isn't merging yet, so the reasoning lives with the branch. Prerequisite, now resolvedThe step-up gate on passkey deletion was circumventable at the endpoint level: Spring Security's The blocker
Consequence: with CONFIG.md currently promises the opposite: that accounts with no passkey stay governed by The fix needs step-up to distinguish "this user could satisfy a configured factor and didn't" from "this user has no way to satisfy any configured factor", and treat the second as a fallback rather than a denial. The framework has no per-factor notion of achievability today, so this is a design addition, not a patch. Also outstanding
AssessmentThe architecture is sound. Freshness is delegated to Spring Security's What's missing is a decision on how step-up behaves for users who cannot satisfy any configured factor. That gates the rest. |
|
Review: Step-Up Re-Authentication (#335) Solid design — expressing step-up as a freshness check on Spring Security's own Two issues worth fixing before merge: 1.
Practical impact: Fix: add 2. Stale javadoc in The class-level "Residual risk (passwordless accounts)" javadoc block (near the top of
That's no longer accurate — this PR gates passkey delete/rename via Minor / non-blocking
Nice work overall — the |
Step-up denied any user who had not recently proven presence, on the assumption they could go and do so. That assumption fails for accounts holding no credential the configured factors accept. UserService.hasPassword() is a null check on the password column, and DSOAuth2UserService/DSOidcUserService create social users without ever setting one, so every OAuth2/OIDC account is "passwordless" here. With user.security.stepUp.enabled=true such an account could never produce a FACTOR_WEBAUTHN, and the else-if holding allowInitialPasswordSetWithoutStepUp was unreachable whenever a StepUpService bean existed, so POST /user/setPassword returned HTTP 401 permanently with no configuration that recovered it. "Sign in with Google, then add a password" was broken. Adds StepUpService.canSatisfyStepUp(User), a default method returning true so existing consumer implementations are unaffected in both source and behavior. The built-in service overrides it: WEBAUTHN needs a registered passkey, PASSWORD needs a password, and factors delivered out of band cannot be ruled out from here so they stay gated. UserAPI treats false as "step-up does not apply" and falls back to the configured default rather than denying. Placing the check on the SPI rather than at the call site keeps it correct for consumer implementations, whose mechanism may not involve passkeys at all and for which a call-site hasCredentials() test would silently skip a control that did apply. ./gradlew check: green on a forced full rerun.
|
Review: WebAuthn step-up (SUF-02) Solid design — the step-up-is-just-re-running-the-login-ceremony approach (no new endpoint/challenge/token) is elegant, and the security fix that fell out of it (FactorAuthorityNameValidator) is a real find: a FACTOR_*-named role/privilege would otherwise be an authentication-bypass vector once MFA or step-up is enabled. Off-by-default, well-tested (unit + autoconfig + integration), and CONFIG.md/MIGRATION.md are thorough. A few things worth a look before merge. Correctness: StepUpConfigProperties is missing @validated, so @min(1) on ttlSeconds likely isn't enforced. See src/main/java/.../security/StepUpConfigProperties.java lines 26-27 and 59-60: @DaTa Spring Boot only validates @ConfigurationProperties classes 'whenever they are annotated with @validated' per the reference docs — the JSR-303 annotations are otherwise inert. The two sibling classes in this same package do it correctly: UserSecurityConfigProperties.java:34-35 and PasswordPolicyConfigProperties.java:24-25 both pair @validated with @ConfigurationProperties. StepUpConfigProperties drops @validated while keeping @min(1), so a misconfigured user.security.stepUp.ttlSeconds=0 (or negative) would silently pass startup instead of failing loudly — which undercuts this PR's own stated goal for StepUpAutoConfiguration.validateFactors(): 'Fails startup rather than at the first gated request: a typo here would otherwise surface as an operation nobody can ever perform.' A ttlSeconds=0 has essentially the same failure mode (the TTL window becomes unsatisfiable in practice) but isn't caught. There's also no test exercising an invalid ttlSeconds, which would have caught this. Recommend adding @validated plus a test alongside the existing shouldFailStartupForUnknownFactor/shouldFailStartupForEmptyFactors cases. Nits (stale Javadoc): A couple of doc comments weren't updated to reflect the new gate and now contradict the code right below them. WebAuthnManagementAPI.java lines 65-70 (class-level 'Residual risk (passwordless accounts)' section) and line 243 ('If the account is passwordless ... this check is a no-op') still describe passkey delete/rename as unconditionally session-only / step-up as unimplemented — but requireCredentialProof a few lines later throws WebAuthnStepUpRequiredException when a StepUpService is configured and unsatisfied. Worth updating to say 'no-op only when no StepUpService is configured.' Also, MfaFilterMergingConfiguration.java line 80 — 'The bean exists only when user.mfa.enabled=true' is now inaccurate; it's also active when step-up alone is enabled (correctly reflected in the class-level Javadoc above it and in the FactorMergingEnabled condition, just not this one line). Design notes (not blockers, just flagging for visibility): The step-up proof is intentionally session+time bound but not single-use or per-action (documented clearly in DSFactorFreshnessStepUpService's class Javadoc and MIGRATION.md). That's a reasonable, disclosed trade-off given the short default TTL, but worth an explicit sign-off since it means any credential-altering call within the window is authorized, not just the one that triggered re-auth. Separately, WebAuthnManagementAPI.requireCredentialProof doesn't call canSatisfyStepUp() before gating delete/rename (unlike UserAPI.setPassword, which does). In practice this looks harmless — an account with no passkey has no credentials to delete/rename in the first place — but it does mean a stale/edge-case call would surface as 401 step-up-required rather than 404 not found; worth a comment noting this is intentional if it is. Finally, per the PR description, the full re-login flow (session fixation, duplicate InteractiveAuthenticationSuccessEvent, login JSON response mid-flow) is explicitly not yet verified against a real browser/demo app — tracked separately, but it's the part of this change with the most unknowns, so I'd treat that follow-up as load-bearing before calling step-up production-ready rather than optional polish. Test coverage: Otherwise strong — freshness/TTL/factor-selection/caller-identity/satisfiability all covered in DSFactorFreshnessStepUpServiceTest, autoconfig backoff/failure modes in StepUpAutoConfigurationTest, the merging-condition interaction in MfaFilterMergingConfigurationTest, and the reserved-name validator in FactorAuthorityNameValidatorTest. The one gap is the ttlSeconds validation case noted above. |
…config Follow-up to 7f84430, from review of that commit. requireCredentialProof did not consult canSatisfyStepUp. The reasoning was that rename and delete imply the user holds a passkey, which is true but answers the wrong question: what matters is whether they hold a credential the *configured* factors accept. validateFactors accepts factors=[PASSWORD] alone, and a passwordless passkey-holder then needs a FACTOR_PASSWORD it can never produce, so rename and non-last delete returned 401 permanently. That is the dead end 7f84430 removed from setPassword, recreated one API over. Also: - canSatisfyStepUp gains the action parameter, matching isStepUpSatisfied, so an implementation whose requirements differ per operation can say so. Free now; breaking for implementors after release. - Startup fails when WEBAUTHN is a step-up factor while WebAuthn is disabled, and PASSWORD logs a warning, mirroring MfaConfiguration. The rejected combination previously started clean and left step-up inert. - A null user keeps the gate rather than releasing it. The SPI says user is never null, and releasing a security gate is the wrong answer to a contract violation. - Locale.ROOT in buildManager and validateFactors. Under a Turkish default locale "authorization_code" uppercased to a dotted I and missed the factor map, failing startup on valid configuration. Tests: the auto-configuration lambda is now pinned (present, absent, and delegating to real credential state), the SPI default is pinned at true so the compatibility promise cannot be flipped silently, and the locale normalization is covered under @ResourceLock(LOCALE). ./gradlew check --rerun-tasks: green. ./gradlew javadoc: clean.
Review: built-in WebAuthn step-up (SUF-02)Overall this is a well-reasoned feature. The threat model is spelled out explicitly (session-cookie-only attacker vs. session-piggybacking attacker), it's off by default with no behavior change, the A few things worth a look before merge: 1. On top of that, this library ships Net effect: 2. 3. Minor: Other notes (no action needed, just confirming reasoning checks out)
|
Groundwork for gating passkey enrollment on recent authentication. A freshness gate can only work if every interactive login leaves a factor to measure; verification found three paths that leave none. Verified in Spring Security 7.1.0 bytecode: the password provider stamps FACTOR_PASSWORD and OAuth2LoginAuthenticationProvider stamps FACTOR_AUTHORIZATION_CODE, but OidcAuthorizationCodeAuthenticationProvider stamps nothing. An OIDC login (Keycloak, or Google configured with the openid scope) therefore produces a session carrying no factor at all. UserService.authWithoutPassword has the same problem for a different reason: it builds its own Authentication rather than running a provider. It has three call sites, and only one is dev-only. The email-verification link and post-registration auto-login are production paths, so without this every newly registered user's first session would carry no factor and they could not enroll their first passkey until logging out and back in. - AuthWithoutPasswordFactor names what each path actually proves: EMAIL_VERIFICATION is a one-time token, REGISTRATION follows a password submitted in that same request, DEV_LOGIN proves nothing and stamps nothing. The single-argument authWithoutPassword delegates to DEV_LOGIN, so existing callers are unchanged. - LoginFactorStamper adds FACTOR_AUTHORIZATION_CODE only when no factor is present at all. The discriminator is deliberately not the token type: on both the OAuth2 and OIDC paths this framework's user services return a DSUserDetails carrying database roles, so the usual OidcUserAuthority versus OAuth2UserAuthority test does not apply here. Checking for the factor itself avoids duplicating the one the OAuth2 provider adds after the mapper runs. - LoginSuccessService applies it, rebuilding only OAuth2AuthenticationToken and logging anything else rather than guessing how to reconstruct it. The context is written back explicitly, since AbstractAuthenticationProcessingFilter saves it before the success handler runs, matching WebAuthnAuthenticationSuccessHandler. No gate yet, so nothing changes for users. ./gradlew check --rerun-tasks: green. ./gradlew javadoc: clean.
|
Review I ran a focused review of this PR (step-up authentication support, issue 335) against the repo conventions in Potential bugs
Minor / style
Test coverage No test currently exercises which factor authority gets stamped on the passwordless registration auto-login path (only Nice work overall — the fail-closed defaults, startup validation of factor names, and the |
Enrolling a passkey was ungated, which meant step-up protected nothing. An attacker holding only a stolen session cookie on a passwordless account could register their own authenticator, assert with it to mint a genuinely fresh FACTOR_WEBAUTHN, and thereby satisfy every gate the feature adds. Worse, enabling step-up *opened* a path: for a social-login account, POST /user/setPassword is closed by allowInitialPasswordSetWithoutStepUp when step-up is off, but reachable via a self-enrolled passkey when it is on. POST /webauthn/register now requires an authentication factor issued within user.security.stepUp.enrollmentTtlSeconds (default 600), applied only while step-up is enabled. This follows GitHub's sudo mode, which asks for a password before adding a security key. Any factor counts, deliberately not the configured stepUp.factors list: with the default [WEBAUTHN] that would demand a passkey in order to register a first passkey, which nobody could satisfy. A plain authority merely named like a factor carries no issue time and can never be fresh, so it cannot forge recency. The window is separate from and longer than ttlSeconds because enrollment follows a login and a decision, whereas a step-up ceremony immediately precedes its operation. Wiring is an authorizeHttpRequests rule, which works because WebAuthnRegistrationFilter is registered after AuthorizationFilter. That ordering is the assumption the whole gate rests on, so WebAuthnEnrollmentGateIntegrationTest exercises the real filter chain rather than the manager alone; disabling the gate fails both of its rejection cases. Also corrects the threat-model claim this work started from. The JavaDoc, MIGRATION.md and the ticket all asserted that an attacker with a session cookie and no authenticator "can never produce a recent factor". That was false while enrollment was open, and the sentence had been copied into three places. It now states what is actually enforced, including the two residuals that remain: same-session piggybacking, and a session stolen within the enrollment window of a real login. ./gradlew check --rerun-tasks: green. ./gradlew javadoc: clean.
Review: built-in WebAuthn step-up (#365)This is a well-reasoned feature with unusually thorough commit messages and good test coverage for the happy paths. I went deep on the freshness/merge mechanics since that's where the security properties live, and found a few issues worth fixing before merge — mostly around the OAuth2/OIDC edges of the "any login is a factor" design. 1.
|
A3 - passkey delete/rename keyed off StepUpService bean presence, so an application that adopted the SPI in 5.3.1 for setPassword would have had two more operations gated on upgrade, with two action values its implementation never expected. The opt-in is now the property. B3 - @min(1) on ttlSeconds was inert because StepUpConfigProperties was not @validated, unlike its siblings. ttlSeconds=0 bound cleanly and made every gated operation deny forever with a debug log as the only signal. Removing @validated again fails the two new tests, so the annotation is what enforces it. enrollmentTtlSeconds is covered too. E3 - FactorAuthorityNameValidator read configuration only, but AuthorityService grants from the role and privilege tables and RolePrivilegeSetupService never deletes. A FACTOR_-prefixed row created under an earlier configuration therefore survived its removal from YAML, kept being granted, and passed the check clean, leaving the MFA bypass the class exists to prevent. It now queries both tables as well, and the error message says to delete the rows rather than only rename the config. The check also moved from @PostConstruct to ContextRefreshedEvent: nothing injects this bean, so under spring.main.lazy-initialization=true it was never constructed and the check silently never ran. The event also orders it after RolePrivilegeSetupService. Matches the pattern that service and MfaConfiguration already use. F1 - the 401 and its step-up-required code had no test, though the distinct code is the entire reason the exception type exists. A second test pins that it does not fall through to the base WebAuthnException handler, which would return 400 with a null error field. Also corrects the property path in the validator's message and in MIGRATION.md: the real key is user.roles.roles-and-privileges, so an operator hit by the failure would have grepped for a name that does not exist. ./gradlew check --rerun-tasks: green. ./gradlew javadoc: clean.
ReviewThis is a well-reasoned, thoroughly-tested implementation of step-up auth (30 new tests, extensive commit-message documentation of the threat model). One finding below looks like a genuine bug that defeats the feature for OAuth2/OIDC users; the rest are minor. 🔴 Bug:
|
Enrolling a passkey grants a durable new way into an account, and it outlives a password change, because session invalidation ends sessions rather than credentials. An attacker who reaches an authenticated session can therefore leave themselves a way back in that survives the victim's response. Until now that enrollment produced no audit record and no notification: Spring Security owns POST /webauthn/register, and nothing in this framework observed it. The framework does own the JPA UserCredentialRepository every enrollment writes through, so the event is published from there and catches any path that registers a credential. The trap, and why the discriminator matters: Spring Security also calls UserCredentialRepository.save() inside authenticate(), to persist the updated signature count. Publishing on every save would email the user on each passkey login. save() already resolves the row with findById(...).orElseGet(new), so an absent row identifies a genuine registration with no extra query. A test pins this by asserting no event on the update path; sabotaging the guard fails it. - WebAuthnCredentialRegisteredEvent carrying the user, credential id, and label - WebAuthnCredentialRegistrationListener publishing a PasskeyRegistration audit event, then emailing the owner - user.webauthn.notifyOnRegistration (default true) gates the email only; the audit event is unconditional, and a mail failure cannot lose it - New mail template and message keys naming the specific risk: a passkey outlives a password change, so both steps are needed to recover This is detective, not preventive. Preventing enrollment from a session-only actor is step-up (#335 / #365), which is off by default and does not yet gate enrollment. This applies whether or not step-up is on. ./gradlew check: green. ./gradlew javadoc: clean.
Implements #335. Off by default; no behavior change until
user.security.stepUp.enabled=true.Approach
Step-up is a freshness requirement on a Spring Security factor, not a bespoke ceremony.
WebAuthnAuthenticationProviderstampsFACTOR_WEBAUTHNwith the moment of assertion, and re-running the ordinary passkey login while already authenticated merges a newly stamped factor onto the session. The ceremony is therefore the login flow the application already has, re-run: no challenge endpoint, no server-side challenge state, no step-up token, and nothing new for clients to implement beyond handling a 401.The framework behavior this rests on is pinned by
WebAuthnStepUpFactorAssumptionsTest(#364).What changed
StepUpConfigProperties:user.security.stepUp.{enabled,ttlSeconds,factors}, defaulting tofalse/120/[WEBAUTHN]. Unknown or empty factor names fail startup rather than producing an operation nobody can perform.DSFactorFreshnessStepUpService: registered under@ConditionalOnMissingBean(StepUpService.class)when enabled. It satisfies the SPI shipped in security: remaining SUF fixes (SUF-02 step-up, SUF-05 reset token, SUF-06 hardening) #334 unchanged, so consumer implementations keep working and still take precedence.StepUpServiceis configured, so existing deployments see no change. Rejections are401with error codestep-up-required.user.mfa.enabled. Without it, a re-assertion replaces the session's authorities instead of refreshing the factor on them.Security fix that fell out of this
FactorAuthorityNameValidatorrejectsFACTOR_-prefixed role and privilege names. A plain granted authority with such a name is indistinguishable from a real factor by name, and database-loaded authorities sort ahead of the stamped one, so:Startup fails when such a name is configured while MFA or step-up is enabled, and logs an error otherwise.
Decisions
Recorded on #335: the proof is bound to user, session, and time, but is not single-use and not per-action. That closes the case the feature exists for, an attacker holding a session cookie and no authenticator. It leaves a narrower one open, an attacker sharing the session piggybacking inside the window, hence the short default TTL. Accounts with no passkey (OAuth-only) stay on
allowInitialPasswordSetWithoutStepUp;AUTHORIZATION_CODEis configurable but not recommended, since refreshing it usually round-trips silently through the identity provider.Verification
./gradlew check: 1204 tests, 0 failures (30 new)../gradlew javadocclean.Not verified in a browser: step-up is a full re-login, so session fixation, the duplicate
InteractiveAuthenticationSuccessEvent, and the login JSON response landing mid-flow all need the demo app. That is devondragon/SpringUserFrameworkDemoApp#75, which also carries the Playwright E2E.