fix: deny Spring Security's built-in WebAuthn credential-delete endpoint (GHSA-3cv9-vgqh-jwpm)

[devondragon]

Summary

Closes the missing-authorization vulnerability in GHSA-3cv9-vgqh-jwpm (CWE-862, medium).

WebSecurityConfig.setupWebAuthn calls http.webAuthn(...), which registers Spring Security's WebAuthnRegistrationFilter and its DELETE /webauthn/register/{id} endpoint. That endpoint deletes a passkey after checking only credential ownership, bypassing every safeguard the framework applies on its own DELETE /user/webauthn/credentials/{id}:

• last-credential lockout protection (passkey-only accounts can be permanently locked out)
• current-password re-authentication
• audit logging

Any authenticated session (stolen cookie, XSS, shared browser) could silently delete a victim's passkeys.

Change

Deny DELETE /webauthn/register/** in the filter chain whenever framework WebAuthn is enabled. The deny rule is registered before anyRequest(), so it is evaluated ahead of WebAuthnRegistrationFilter (added after AuthorizationFilter) and applies in both deny and allow modes. POST /webauthn/register is left untouched.

Consumers relying on the Spring endpoint should migrate to DELETE /user/webauthn/credentials/{id}.

Tests

Added a case to WebAuthnFeatureEnabledIntegrationTest asserting an authenticated DELETE /webauthn/register/{id} returns 403 and the credential survives. Full ./gradlew test suite passes.

Follow-ups (not in this PR)

• MIGRATION.md / CHANGELOG note directing consumers to the framework endpoint.
• Demo-app validation per the release protocol (publishLocal + demo tests + WebAuthn/MFA Playwright) before release.
• Fill in the advisory's patched version and publish after release. The affected range starts at 4.2.0 (when WebAuthn was introduced), so the 3.6.x maintenance line is unaffected.

[Copilot]

Pull request overview

This PR addresses GHSA-3cv9-vgqh-jwpm by preventing use of Spring Security’s built-in WebAuthn credential deletion endpoint (DELETE /webauthn/register/{id}) when the framework’s WebAuthn feature is enabled, ensuring credential deletion only occurs via the framework-managed endpoint that enforces additional safeguards.

Changes:

• Deny DELETE /webauthn/register/** in the security filter chain when WebAuthn is enabled.
• Add an integration test asserting the endpoint returns 403 and does not delete the credential.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/main/java/com/digitalsanctuary/spring/user/security/WebSecurityConfig.java	Adds an authorization rule to deny Spring Security’s built-in WebAuthn credential-delete endpoint when WebAuthn is enabled.
src/test/java/com/digitalsanctuary/spring/user/api/WebAuthnFeatureEnabledIntegrationTest.java	Adds an integration test to verify the denied endpoint returns 403 and preserves the credential.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[claude]

Review

Solid, well-scoped fix for a real CWE-862 gap. http.webAuthn(...) does register Spring Security's own WebAuthnRegistrationFilter/DELETE /webauthn/register/{id}, which only checks credential ownership and bypasses this library's lockout protection, re-auth, and audit logging on DELETE /user/webauthn/credentials/{id} — closing that off with a denyAll() is the right call.

What's good:

• Scoping the deny to HttpMethod.DELETE only (leaving POST /webauthn/register untouched) is precise — doesn't break passkey registration.
• Registering the rule before the defaultAction block is not just tidy, it's required: Spring rejects requestMatchers() calls after anyRequest(), and under defaultAction=allow the later anyRequest().permitAll() would otherwise re-expose the endpoint. Applying the deny unconditionally (independent of defaultAction) is correct.
• The regression test drives the real filter chain (@AutoConfigureMockMvc + the actual SecurityFilterChain bean, not a mock), so it's a genuine end-to-end check of the bypass path rather than a unit test of the matcher config in isolation.

A few things worth considering:

1. Test coverage gap for the scenario the PR narrative leads with. The write-up specifically calls out that this fix matters most under defaultAction=allow, where anyRequest().permitAll() would otherwise expose the endpoint. But WebAuthnFeatureEnabledIntegrationTest only runs against the test suite's default defaultAction=deny profile (application-test.yml) — there's no test with WebAuthn enabled and defaultAction=allow together anywhere in the suite (WebSecurityAuthorizationAllowTest doesn't enable WebAuthn). Worth adding a case combining user.webauthn.enabled=true with user.security.defaultAction=allow to actually exercise the scenario the fix is motivated by, rather than relying on the deny-mode test to stand in for it.

2. Only the authenticated-owner path is tested. It'd be worth pinning down (with a test) what happens for an unauthenticated DELETE /webauthn/register/{id} — ExceptionTranslationFilter typically routes an AccessDeniedException for an anonymous principal through the AuthenticationEntryPoint (redirect/401) rather than a bare 403, so the anonymous-caller contract is a little different from the authenticated 403 case this PR covers. Documenting that in a test would remove any ambiguity for future readers.

3. The fix's correctness leans on filter ordering internal to spring-security-webauthn (that WebAuthnRegistrationFilter runs after AuthorizationFilter evaluates authorizeHttpRequests, so the deny rule intercepts the request before the filter acts on it). That's outside this library's control and could shift in a future Spring Security release. The new integration test is what makes this safe long-term since it runs through the real chain and would fail loudly if that ever changes — it'd help future maintainers to say so explicitly in a comment on the test (e.g. "this test guards a filter-ordering assumption in spring-security-webauthn; a failure here likely means that assumption changed upstream"), so a break reads as expected rather than mysterious.

4. Minor nit: webAuthnConfigProperties.isEnabled() is checked twice (once to call setupWebAuthn, again for the new deny block a few lines later). Purely cosmetic — hoisting to a local boolean webAuthnEnabled would read slightly cleaner, no functional impact.

Nothing here blocks merging; the core fix looks correct and the reasoning in the comments/commit message is unusually thorough. The two test-coverage suggestions (#1 especially) would make the safety net match the threat model actually described in the PR.

[devondragon]

Ticket-Grounded Review: GHSA-3cv9-vgqh-jwpm

Ground truth for intent is security advisory GHSA-3cv9-vgqh-jwpm (no numeric issue). AC inferred from the advisory's Impact + Planned fix.

Acceptance Criteria

• AC1 — DELETE /webauthn/register/** denied when framework WebAuthn is enabled — Addressed
• AC2 — deny precedes anyRequest(), applies in both deny and allow modes — Addressed (now tested in both modes)
• AC3 — framework's own DELETE /user/webauthn/credentials/{id} still works, deny is tested — Addressed

Ticket-Scope Drift

• In-scope: WebSecurityConfig.java (deny rule) → AC1/AC2; two test files → AC2/AC3. No out-of-scope changes.
• Missing from ticket: none. POST /webauthn/register correctly left intact. Docs / advisory-publish are release-time follow-ups, not code gaps.

Code review findings

• Verdict: PASS. No critical or unresolved findings.
• One AC gap, closed during review: AC2's allow-mode branch was structurally correct but untested. Added WebAuthnRegistrationDeleteAllowModeTest (anonymous DELETE → 302 login redirect, authenticated DELETE → 403). Verified (2 tests, 0 failures) and committed as 0274665.
• Security verification (bytecode-level): decompiled WebAuthnRegistrationFilter. On spring-security-webauthn 7.0.4 (the 3.5/Java-17 line), the filter's removeCredential deletes by ID with no ownership check, and this project's JpaUserCredentialRepository.delete also does an unconditional deleteById — so pre-fix, any authenticated caller (or any anonymous caller in allow mode) could delete another user's passkey. 7.1.0 already defaults this manager to denyAll, but the authorizeHttpRequests fix is version-independent and covers both support lines. Severity justified.
• Low/INFO (not actioned): webAuthnConfigProperties.isEnabled() is checked twice (once for setupWebAuthn, once for the deny rule). Purely stylistic; left as-is to avoid touching unrelated code.

Verification

./gradlew test — 1175 tests, 0 failures, 0 errors across 276 classes, including the pre-existing allow/deny authorization suites (no regression).

Verdict: clean. No out-of-scope changes, no missing AC. Three commits: the fix, the original deny test, and the review-added allow-mode test.

Remaining are non-code release follow-ups (already noted on this PR): MIGRATION/CHANGELOG note, demo-app Playwright validation, and publishing the advisory's patched version after release.

Reviewed via /ccmagic:review-ticket.

[claude]

Review

This is a well-scoped fix for GHSA-3cv9-vgqh-jwpm. Traced the change through WebSecurityConfig and Spring Security's HttpSecurity authorization DSL to confirm correctness — summary below.

Correctness

• Ordering is correct. Spring Security's authorizeHttpRequests() reuses the same AuthorizeHttpRequestsConfigurer instance across multiple calls on the same HttpSecurity, appending matchers to one ordered list rather than replacing it. Since the new DELETE /webauthn/register/** → denyAll() rule is registered before both the deny-mode and allow-mode branches (which each end in anyRequest()), it's evaluated first and wins regardless of defaultAction. This matches the PR's stated reasoning and I don't see a hole in it.
• Scope is tight. The matcher is method-specific (DELETE only) and path-specific (/webauthn/register/**), so POST /webauthn/register (the actual passkey-creation ceremony) and WebAuthnAuthenticationFilter's login flow (/login/webauthn) are untouched. Confirmed no template/JS in src/main references the built-in DELETE /webauthn/register/{id} endpoint, so there's no self-inflicted regression — the framework's own UI already uses DELETE /user/webauthn/credentials/{id}.
• allow-mode default config check: /webauthn/register isn't in the default unprotectedURIs list (dsspringuserconfig.properties), so under deny mode the endpoint already required authentication before this fix — the real exposure was allow mode's anyRequest().permitAll() fallback for anonymous callers, exactly as the PR description says. Good that the second commit added coverage for that specific case rather than stopping at the deny-mode test.

Test coverage

• Both new tests are meaningful and non-redundant: one locks in deny-mode (extends the existing WebAuthnFeatureEnabledIntegrationTest), the other adds a dedicated allow-mode class covering both anonymous (302, via the existing AuthenticationEntryPoint) and authenticated (403) callers. The anonymous-redirect expectation (is3xxRedirection()) is consistent with the existing convention in WebSecurityAuthorizationAllowTest.
• Both tests assert the credential still exists after the denied call, not just the status code — good, since a status-code-only assertion wouldn't actually prove the delete was blocked.

Minor / non-blocking

• The new block re-checks webAuthnConfigProperties.isEnabled() immediately after the if (webAuthnConfigProperties.isEnabled()) { setupWebAuthn(http); } block above it (lines ~168 and ~180). Purely cosmetic, but folding the deny rule into setupWebAuthn(http) itself (or the same if) would remove the duplicate condition and keep all WebAuthn wiring in one place.
• Agreed with the PR's own follow-up list — the MIGRATION.md/CHANGELOG note for consumers who might currently depend on the built-in endpoint seems worth doing in the same release rather than a true follow-up, since it's a behavior-breaking (albeit correct) change for anyone unknowingly relying on it.

Security

The fix directly addresses the CWE-862 gap described in the advisory (ownership-only check bypassing lockout protection, re-auth, and audit logging). Denying at the filter-chain level rather than patching the built-in filter's behavior is the right call — it can't be undermined by future Spring Security changes to that filter's internals, and it fails closed if http.webAuthn()'s registration filter is ever registered on additional paths.

No concerns blocking merge. Nice, minimal, well-tested fix.