fix: reject empty credential fields with 400 instead of 500
Add an explicit non-empty check before encrypting user-submitted email/password fields in the create and update user routes, so a request that bypasses the HTML `required` attribute gets a clean 400 instead of an unhandled ValueError from CredentialCipher.encrypt propagating as a 500. Applies to all four credential fields on create, and to the two email fields on update (the password-blank- means-keep-existing behavior on update is unchanged). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -24,6 +24,14 @@ def _cipher(request: Request) -> CredentialCipher:
|
|||||||
return CredentialCipher(request.app.state.settings.credential_encryption_key)
|
return CredentialCipher(request.app.state.settings.credential_encryption_key)
|
||||||
|
|
||||||
|
|
||||||
|
def _require_non_empty(value: str, field_name: str) -> None:
|
||||||
|
if not value.strip():
|
||||||
|
raise HTTPException(
|
||||||
|
status_code=status.HTTP_400_BAD_REQUEST,
|
||||||
|
detail=f"{field_name} must not be empty",
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
@router.get("/login", response_class=HTMLResponse)
|
@router.get("/login", response_class=HTMLResponse)
|
||||||
def login_page(request: Request):
|
def login_page(request: Request):
|
||||||
return templates.TemplateResponse(request, "login.html", {"csrf_token": ensure_csrf_token(request)})
|
return templates.TemplateResponse(request, "login.html", {"csrf_token": ensure_csrf_token(request)})
|
||||||
@@ -81,14 +89,18 @@ def create_user(
|
|||||||
request: Request,
|
request: Request,
|
||||||
csrf_token: str = Form(...),
|
csrf_token: str = Form(...),
|
||||||
name: str = Form(...),
|
name: str = Form(...),
|
||||||
mywhoosh_email: str = Form(...),
|
mywhoosh_email: str = Form(""),
|
||||||
mywhoosh_password: str = Form(...),
|
mywhoosh_password: str = Form(""),
|
||||||
garmin_email: str = Form(...),
|
garmin_email: str = Form(""),
|
||||||
garmin_password: str = Form(...),
|
garmin_password: str = Form(""),
|
||||||
enabled: str | None = Form(None),
|
enabled: str | None = Form(None),
|
||||||
):
|
):
|
||||||
require_admin(request)
|
require_admin(request)
|
||||||
validate_csrf(request, csrf_token)
|
validate_csrf(request, csrf_token)
|
||||||
|
_require_non_empty(mywhoosh_email, "mywhoosh_email")
|
||||||
|
_require_non_empty(mywhoosh_password, "mywhoosh_password")
|
||||||
|
_require_non_empty(garmin_email, "garmin_email")
|
||||||
|
_require_non_empty(garmin_password, "garmin_password")
|
||||||
form = UserFormData(
|
form = UserFormData(
|
||||||
name=name,
|
name=name,
|
||||||
mywhoosh_email=mywhoosh_email,
|
mywhoosh_email=mywhoosh_email,
|
||||||
@@ -152,14 +164,16 @@ def update_user(
|
|||||||
user_id: int,
|
user_id: int,
|
||||||
csrf_token: str = Form(...),
|
csrf_token: str = Form(...),
|
||||||
name: str = Form(...),
|
name: str = Form(...),
|
||||||
mywhoosh_email: str = Form(...),
|
mywhoosh_email: str = Form(""),
|
||||||
mywhoosh_password: str = Form(""),
|
mywhoosh_password: str = Form(""),
|
||||||
garmin_email: str = Form(...),
|
garmin_email: str = Form(""),
|
||||||
garmin_password: str = Form(""),
|
garmin_password: str = Form(""),
|
||||||
enabled: str | None = Form(None),
|
enabled: str | None = Form(None),
|
||||||
):
|
):
|
||||||
require_admin(request)
|
require_admin(request)
|
||||||
validate_csrf(request, csrf_token)
|
validate_csrf(request, csrf_token)
|
||||||
|
_require_non_empty(mywhoosh_email, "mywhoosh_email")
|
||||||
|
_require_non_empty(garmin_email, "garmin_email")
|
||||||
form = UserFormData(
|
form = UserFormData(
|
||||||
name=name,
|
name=name,
|
||||||
mywhoosh_email=mywhoosh_email,
|
mywhoosh_email=mywhoosh_email,
|
||||||
|
|||||||
@@ -234,3 +234,111 @@ def test_dashboard_lists_created_user_without_secrets(client: TestClient) -> Non
|
|||||||
assert "Max" in response.text
|
assert "Max" in response.text
|
||||||
assert "mw-secret" not in response.text
|
assert "mw-secret" not in response.text
|
||||||
assert "garmin-secret" not in response.text
|
assert "garmin-secret" not in response.text
|
||||||
|
|
||||||
|
|
||||||
|
def _create_payload(**overrides: str) -> dict:
|
||||||
|
payload = {
|
||||||
|
"name": "Max",
|
||||||
|
"mywhoosh_email": "max@example.com",
|
||||||
|
"mywhoosh_password": "mw-secret",
|
||||||
|
"garmin_email": "max-garmin@example.com",
|
||||||
|
"garmin_password": "garmin-secret",
|
||||||
|
"enabled": "on",
|
||||||
|
}
|
||||||
|
payload.update(overrides)
|
||||||
|
return payload
|
||||||
|
|
||||||
|
|
||||||
|
def test_create_user_rejects_empty_mywhoosh_email(client: TestClient) -> None:
|
||||||
|
login(client)
|
||||||
|
page = client.get("/users/new")
|
||||||
|
csrf = extract_csrf(page.text)
|
||||||
|
response = client.post(
|
||||||
|
"/users",
|
||||||
|
data={"csrf_token": csrf, **_create_payload(mywhoosh_email="")},
|
||||||
|
)
|
||||||
|
assert response.status_code == 400
|
||||||
|
|
||||||
|
|
||||||
|
def test_create_user_rejects_empty_mywhoosh_password(client: TestClient) -> None:
|
||||||
|
login(client)
|
||||||
|
page = client.get("/users/new")
|
||||||
|
csrf = extract_csrf(page.text)
|
||||||
|
response = client.post(
|
||||||
|
"/users",
|
||||||
|
data={"csrf_token": csrf, **_create_payload(mywhoosh_password="")},
|
||||||
|
)
|
||||||
|
assert response.status_code == 400
|
||||||
|
|
||||||
|
|
||||||
|
def test_create_user_rejects_empty_garmin_email(client: TestClient) -> None:
|
||||||
|
login(client)
|
||||||
|
page = client.get("/users/new")
|
||||||
|
csrf = extract_csrf(page.text)
|
||||||
|
response = client.post(
|
||||||
|
"/users",
|
||||||
|
data={"csrf_token": csrf, **_create_payload(garmin_email="")},
|
||||||
|
)
|
||||||
|
assert response.status_code == 400
|
||||||
|
|
||||||
|
|
||||||
|
def test_create_user_rejects_empty_garmin_password(client: TestClient) -> None:
|
||||||
|
login(client)
|
||||||
|
page = client.get("/users/new")
|
||||||
|
csrf = extract_csrf(page.text)
|
||||||
|
response = client.post(
|
||||||
|
"/users",
|
||||||
|
data={"csrf_token": csrf, **_create_payload(garmin_password="")},
|
||||||
|
)
|
||||||
|
assert response.status_code == 400
|
||||||
|
|
||||||
|
|
||||||
|
def test_create_user_rejects_whitespace_only_email(client: TestClient) -> None:
|
||||||
|
login(client)
|
||||||
|
page = client.get("/users/new")
|
||||||
|
csrf = extract_csrf(page.text)
|
||||||
|
response = client.post(
|
||||||
|
"/users",
|
||||||
|
data={"csrf_token": csrf, **_create_payload(mywhoosh_email=" ")},
|
||||||
|
)
|
||||||
|
assert response.status_code == 400
|
||||||
|
|
||||||
|
|
||||||
|
def test_update_user_rejects_empty_mywhoosh_email(client: TestClient) -> None:
|
||||||
|
login(client)
|
||||||
|
user_id = create_user_via_http(client)
|
||||||
|
edit_page = client.get(f"/users/{user_id}/edit")
|
||||||
|
csrf = extract_csrf(edit_page.text)
|
||||||
|
response = client.post(
|
||||||
|
f"/users/{user_id}",
|
||||||
|
data={
|
||||||
|
"csrf_token": csrf,
|
||||||
|
"name": "Max",
|
||||||
|
"mywhoosh_email": "",
|
||||||
|
"mywhoosh_password": "",
|
||||||
|
"garmin_email": "max-garmin@example.com",
|
||||||
|
"garmin_password": "",
|
||||||
|
"enabled": "on",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
assert response.status_code == 400
|
||||||
|
|
||||||
|
|
||||||
|
def test_update_user_rejects_empty_garmin_email(client: TestClient) -> None:
|
||||||
|
login(client)
|
||||||
|
user_id = create_user_via_http(client)
|
||||||
|
edit_page = client.get(f"/users/{user_id}/edit")
|
||||||
|
csrf = extract_csrf(edit_page.text)
|
||||||
|
response = client.post(
|
||||||
|
f"/users/{user_id}",
|
||||||
|
data={
|
||||||
|
"csrf_token": csrf,
|
||||||
|
"name": "Max",
|
||||||
|
"mywhoosh_email": "max@example.com",
|
||||||
|
"mywhoosh_password": "",
|
||||||
|
"garmin_email": "",
|
||||||
|
"garmin_password": "",
|
||||||
|
"enabled": "on",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
assert response.status_code == 400
|
||||||
|
|||||||
Reference in New Issue
Block a user