fix(security): POST WebAuthn login-begin; render JS errors as text
Two WebAuthn client/route hardening fixes: - GET /login/webauthn/begin wrote a challenge to the session on a safe method (login-flow DoS / CSRF surface). Make it POST with CSRF, matching the registration-begin endpoint; update webauthn.js and tests accordingly. - webauthn.js rendered dynamic error text (err.message, server error fields) via innerHTML — an XSS-prone sink. Add showAlert() that sets textContent; route all dynamic error messages through it. The trusted server-rendered credentials partial is still injected as markup. Refs: porchlight-cog, porchlight-t7y Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
1571706d21
commit
c175633980
3 changed files with 28 additions and 13 deletions
|
|
@ -157,7 +157,7 @@ async def register_magic_link(request: Request, token: str) -> Response:
|
|||
return RedirectResponse("/manage/credentials?setup=1", status_code=303)
|
||||
|
||||
|
||||
@router.get("/login/webauthn/begin")
|
||||
@router.post("/login/webauthn/begin")
|
||||
async def login_webauthn_begin(request: Request) -> Response:
|
||||
webauthn_service = request.app.state.webauthn_service
|
||||
|
||||
|
|
|
|||
|
|
@ -19,6 +19,16 @@ function getCsrfToken() {
|
|||
return meta ? meta.getAttribute('content') : '';
|
||||
}
|
||||
|
||||
// Render an alert with the message as text (never as HTML) to avoid XSS.
|
||||
function showAlert(el, message) {
|
||||
if (!el) return;
|
||||
el.replaceChildren();
|
||||
const div = document.createElement('div');
|
||||
div.setAttribute('role', 'alert');
|
||||
div.textContent = message;
|
||||
el.appendChild(div);
|
||||
}
|
||||
|
||||
async function beginRegistration() {
|
||||
const statusEl = document.getElementById('webauthn-status');
|
||||
|
||||
|
|
@ -29,7 +39,7 @@ async function beginRegistration() {
|
|||
headers: { 'Content-Type': 'application/json', 'X-CSRF-Token': getCsrfToken() },
|
||||
});
|
||||
if (!beginRes.ok) {
|
||||
if (statusEl) statusEl.innerHTML = '<div role="alert">Failed to start registration</div>';
|
||||
showAlert(statusEl, 'Failed to start registration');
|
||||
return;
|
||||
}
|
||||
const options = await beginRes.json();
|
||||
|
|
@ -74,7 +84,7 @@ async function beginRegistration() {
|
|||
if (statusEl) statusEl.innerHTML = text;
|
||||
}
|
||||
} catch (err) {
|
||||
if (statusEl) statusEl.innerHTML = '<div role="alert">Registration failed: ' + err.message + '</div>';
|
||||
showAlert(statusEl, 'Registration failed: ' + err.message);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -83,10 +93,13 @@ async function beginAuthentication() {
|
|||
|
||||
try {
|
||||
// Step 1: Get options from server (no username needed)
|
||||
const beginRes = await fetch('/login/webauthn/begin');
|
||||
const beginRes = await fetch('/login/webauthn/begin', {
|
||||
method: 'POST',
|
||||
headers: { 'X-CSRF-Token': getCsrfToken() },
|
||||
});
|
||||
if (!beginRes.ok) {
|
||||
const data = await beginRes.json();
|
||||
if (statusEl) statusEl.innerHTML = '<div role="alert">' + (data.error || 'Failed to start authentication') + '</div>';
|
||||
showAlert(statusEl, data.error || 'Failed to start authentication');
|
||||
return;
|
||||
}
|
||||
const options = await beginRes.json();
|
||||
|
|
@ -129,10 +142,10 @@ async function beginAuthentication() {
|
|||
window.location.href = data.redirect || '/manage/credentials';
|
||||
} else {
|
||||
const data = await completeRes.json().catch(function () { return {}; });
|
||||
if (statusEl) statusEl.innerHTML = '<div role="alert">' + (data.error || 'Authentication failed') + '</div>';
|
||||
showAlert(statusEl, data.error || 'Authentication failed');
|
||||
}
|
||||
} catch (err) {
|
||||
if (statusEl) statusEl.innerHTML = '<div role="alert">Authentication failed: ' + err.message + '</div>';
|
||||
showAlert(statusEl, 'Authentication failed: ' + err.message);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -46,10 +46,12 @@ async def _setup_user_with_webauthn(
|
|||
|
||||
|
||||
async def test_webauthn_login_begin_returns_options(client: AsyncClient) -> None:
|
||||
"""Begin is now GET with no username — returns options with empty allowCredentials."""
|
||||
"""Begin is a POST (mutates session) with no username — returns options
|
||||
with empty allowCredentials."""
|
||||
await _setup_user_with_webauthn(client)
|
||||
|
||||
res = await client.get("/login/webauthn/begin")
|
||||
token = await get_csrf_token(client)
|
||||
res = await client.post("/login/webauthn/begin", headers={"X-CSRF-Token": token})
|
||||
assert res.status_code == 200
|
||||
data = res.json()
|
||||
assert "publicKey" in data
|
||||
|
|
@ -59,7 +61,8 @@ async def test_webauthn_login_begin_returns_options(client: AsyncClient) -> None
|
|||
|
||||
|
||||
async def test_webauthn_login_begin_has_user_verification_preferred(client: AsyncClient) -> None:
|
||||
res = await client.get("/login/webauthn/begin")
|
||||
token = await get_csrf_token(client)
|
||||
res = await client.post("/login/webauthn/begin", headers={"X-CSRF-Token": token})
|
||||
assert res.status_code == 200
|
||||
data = res.json()
|
||||
assert data["publicKey"]["userVerification"] == "preferred"
|
||||
|
|
@ -81,10 +84,9 @@ async def test_webauthn_login_complete_returns_json_redirect(client: AsyncClient
|
|||
_userid, _pk, _credential_id, _att = await _setup_user_with_webauthn(client)
|
||||
|
||||
# Begin to get state into session
|
||||
res1 = await client.get("/login/webauthn/begin")
|
||||
assert res1.status_code == 200
|
||||
|
||||
token = await get_csrf_token(client)
|
||||
res1 = await client.post("/login/webauthn/begin", headers={"X-CSRF-Token": token})
|
||||
assert res1.status_code == 200
|
||||
# We can't easily complete the full assertion without browser interaction,
|
||||
# but we verify the endpoint returns 400 JSON (not HTML) for bad assertions
|
||||
res2 = await client.post(
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue