Studio: reject whitespace-only passwords (#7341)
* Studio: reject whitespace-only passwords * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: reject any whitespace in passwords * Studio: surface whitespace error in setup form, isolate auth test import --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Daniel Han <danielhanchen@gmail.com>
This commit is contained in:
parent
430ada617a
commit
13c7db1965
9 changed files with 136 additions and 6 deletions
|
|
@ -236,6 +236,10 @@ def prompt_for_password_change(
|
|||
out.write(f"Password must be at least {min_length} characters; try again.\n")
|
||||
out.flush()
|
||||
continue
|
||||
if any(ch.isspace() for ch in new_password):
|
||||
out.write("Password cannot contain spaces; try again.\n")
|
||||
out.flush()
|
||||
continue
|
||||
if is_current_password(new_password):
|
||||
out.write(
|
||||
"New password must differ from the current bootstrap password; try again.\n"
|
||||
|
|
|
|||
|
|
@ -494,6 +494,11 @@ async def change_password(
|
|||
status_code = status.HTTP_401_UNAUTHORIZED,
|
||||
detail = "Current password is incorrect",
|
||||
)
|
||||
if any(ch.isspace() for ch in payload.new_password):
|
||||
raise HTTPException(
|
||||
status_code = status.HTTP_400_BAD_REQUEST,
|
||||
detail = "New password cannot contain spaces",
|
||||
)
|
||||
if payload.current_password == payload.new_password:
|
||||
raise HTTPException(
|
||||
status_code = status.HTTP_400_BAD_REQUEST,
|
||||
|
|
|
|||
|
|
@ -1244,6 +1244,13 @@ def _apply_supplied_password(password_value: "Optional[str]") -> None:
|
|||
flush = True,
|
||||
)
|
||||
sys.exit(1)
|
||||
if any(ch.isspace() for ch in supplied):
|
||||
print(
|
||||
"Error: password cannot contain spaces; not starting.",
|
||||
file = sys.stderr,
|
||||
flush = True,
|
||||
)
|
||||
sys.exit(1)
|
||||
if _is_current_password(supplied):
|
||||
print(
|
||||
"Error: the new password must differ from the current bootstrap "
|
||||
|
|
|
|||
75
studio/backend/tests/test_change_password_policy.py
Normal file
75
studio/backend/tests/test_change_password_policy.py
Normal file
|
|
@ -0,0 +1,75 @@
|
|||
# SPDX-License-Identifier: AGPL-3.0-only
|
||||
# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0
|
||||
|
||||
import asyncio
|
||||
import importlib.util
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
from fastapi import HTTPException
|
||||
|
||||
_BACKEND_ROOT = Path(__file__).resolve().parents[1]
|
||||
if str(_BACKEND_ROOT) not in sys.path:
|
||||
sys.path.insert(0, str(_BACKEND_ROOT))
|
||||
|
||||
from models.auth import ChangePasswordRequest # noqa: E402
|
||||
|
||||
# Load routes/auth.py directly so collection does not execute routes/__init__.py,
|
||||
# which pulls in the heavy training/models/inference routers.
|
||||
_route_path = _BACKEND_ROOT / "routes" / "auth.py"
|
||||
_spec = importlib.util.spec_from_file_location("_change_password_route", _route_path)
|
||||
assert _spec is not None and _spec.loader is not None
|
||||
auth_routes = importlib.util.module_from_spec(_spec)
|
||||
_spec.loader.exec_module(auth_routes)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def _user(monkeypatch):
|
||||
monkeypatch.setattr(
|
||||
auth_routes.storage,
|
||||
"get_user_and_secret",
|
||||
lambda username: ("salt", "hash", "jwt-secret", False),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
auth_routes.hashing,
|
||||
"verify_password",
|
||||
lambda password, salt, pwd_hash: password == "bootstrap-pw",
|
||||
)
|
||||
|
||||
|
||||
def _change(new_password):
|
||||
payload = ChangePasswordRequest(
|
||||
current_password = "bootstrap-pw",
|
||||
new_password = new_password,
|
||||
)
|
||||
return asyncio.run(auth_routes.change_password(payload, None, "unsloth"))
|
||||
|
||||
|
||||
def test_rejects_whitespace_only_password(_user):
|
||||
with pytest.raises(HTTPException) as excinfo:
|
||||
_change(" " * 8)
|
||||
assert excinfo.value.status_code == 400
|
||||
assert "spaces" in excinfo.value.detail
|
||||
|
||||
|
||||
def test_rejects_tabs_and_spaces_password(_user):
|
||||
with pytest.raises(HTTPException) as excinfo:
|
||||
_change(" \t \t \t \t ")
|
||||
assert excinfo.value.status_code == 400
|
||||
|
||||
|
||||
def test_rejects_password_containing_spaces(_user):
|
||||
with pytest.raises(HTTPException) as excinfo:
|
||||
_change("correct horse battery")
|
||||
assert excinfo.value.status_code == 400
|
||||
assert "spaces" in excinfo.value.detail
|
||||
|
||||
|
||||
def test_allows_password_without_spaces(_user, monkeypatch):
|
||||
monkeypatch.setattr(auth_routes.storage, "update_password", lambda *args, **kwargs: True)
|
||||
monkeypatch.setattr(auth_routes, "create_access_token", lambda subject: "at")
|
||||
monkeypatch.setattr(auth_routes, "create_refresh_token", lambda subject: "rt")
|
||||
token = _change("correct-horse-battery")
|
||||
assert token.access_token == "at"
|
||||
assert token.must_change_password is False
|
||||
|
|
@ -183,6 +183,22 @@ def test_loop_short_password_reprompts(monkeypatch):
|
|||
assert "at least 8 characters" in out
|
||||
|
||||
|
||||
def test_loop_whitespace_only_reprompts(monkeypatch):
|
||||
ok, applied, out = _run_loop(monkeypatch, _keys(" " * 8, "long-enough-pw", "long-enough-pw"))
|
||||
assert ok is True
|
||||
assert applied == ["long-enough-pw"]
|
||||
assert "contain spaces" in out
|
||||
|
||||
|
||||
def test_loop_password_with_inner_space_reprompts(monkeypatch):
|
||||
ok, applied, out = _run_loop(
|
||||
monkeypatch, _keys("has space pw", "long-enough-pw", "long-enough-pw")
|
||||
)
|
||||
assert ok is True
|
||||
assert applied == ["long-enough-pw"]
|
||||
assert "contain spaces" in out
|
||||
|
||||
|
||||
def test_loop_rejects_current_password(monkeypatch):
|
||||
ok, applied, out = _run_loop(
|
||||
monkeypatch, _keys("bootstrap-pw", "fresh-password", "fresh-password")
|
||||
|
|
|
|||
|
|
@ -196,8 +196,10 @@ export function AuthForm({ mode }: AuthFormProps): ReactElement | null {
|
|||
!isLoginMode &&
|
||||
(currentPassword.length < 8 ||
|
||||
newPassword.length < 8 ||
|
||||
/\s/.test(newPassword) ||
|
||||
newPassword !== confirmPassword ||
|
||||
currentPassword === newPassword);
|
||||
const showWhitespaceWarning = !isLoginMode && /\s/.test(newPassword);
|
||||
const showPasswordMismatchWarning =
|
||||
!isLoginMode &&
|
||||
newPassword.length > 0 &&
|
||||
|
|
@ -222,6 +224,10 @@ export function AuthForm({ mode }: AuthFormProps): ReactElement | null {
|
|||
setError("New password must be at least 8 characters.");
|
||||
return;
|
||||
}
|
||||
if (/\s/.test(newPassword)) {
|
||||
setError("New password cannot contain spaces.");
|
||||
return;
|
||||
}
|
||||
if (newPassword !== confirmPassword) {
|
||||
setError("Passwords do not match.");
|
||||
return;
|
||||
|
|
@ -425,13 +431,17 @@ export function AuthForm({ mode }: AuthFormProps): ReactElement | null {
|
|||
</div>
|
||||
<p
|
||||
className={`min-h-4 text-xs ${
|
||||
showPasswordMismatchWarning ? "text-destructive" : "text-muted-foreground"
|
||||
showWhitespaceWarning || showPasswordMismatchWarning
|
||||
? "text-destructive"
|
||||
: "text-muted-foreground"
|
||||
}`}
|
||||
aria-live="polite"
|
||||
>
|
||||
{showPasswordMismatchWarning
|
||||
? "Please ensure passwords match."
|
||||
: "Must be at least 8 characters."}
|
||||
{showWhitespaceWarning
|
||||
? "New password cannot contain spaces."
|
||||
: showPasswordMismatchWarning
|
||||
? "Please ensure passwords match."
|
||||
: "Must be at least 8 characters."}
|
||||
</p>
|
||||
</>
|
||||
)}
|
||||
|
|
|
|||
|
|
@ -78,6 +78,9 @@ function passwordValidationMessage(
|
|||
minLength: MIN_PASSWORD_LENGTH,
|
||||
});
|
||||
}
|
||||
if (/\s/.test(nextPassword)) {
|
||||
return t("settings.general.passwordDialog.newHasSpaces");
|
||||
}
|
||||
if (nextPassword !== confirmPassword) {
|
||||
return t("settings.general.passwordDialog.mismatch");
|
||||
}
|
||||
|
|
@ -160,6 +163,7 @@ export function ChangePasswordDialog() {
|
|||
|
||||
const currentTooShort = hasStartedTooShortPassword(current);
|
||||
const nextTooShort = hasStartedTooShortPassword(next);
|
||||
const nextHasSpaces = /\s/.test(next);
|
||||
const mismatch = confirm.length > 0 && next !== confirm;
|
||||
const samePassword = hasReusablePassword(current, next);
|
||||
const validationMessage = passwordValidationMessage(
|
||||
|
|
@ -279,13 +283,15 @@ export function ChangePasswordDialog() {
|
|||
minLength={MIN_PASSWORD_LENGTH}
|
||||
disabled={submitting}
|
||||
/>
|
||||
{nextTooShort || samePassword ? (
|
||||
{nextTooShort || nextHasSpaces || samePassword ? (
|
||||
<p className="text-xs text-destructive" aria-live="polite">
|
||||
{nextTooShort
|
||||
? t("settings.general.passwordDialog.newTooShort", {
|
||||
minLength: MIN_PASSWORD_LENGTH,
|
||||
})
|
||||
: t("settings.general.passwordDialog.samePassword")}
|
||||
: nextHasSpaces
|
||||
? t("settings.general.passwordDialog.newHasSpaces")
|
||||
: t("settings.general.passwordDialog.samePassword")}
|
||||
</p>
|
||||
) : null}
|
||||
</div>
|
||||
|
|
|
|||
|
|
@ -196,6 +196,7 @@ export const en = {
|
|||
currentTooShort:
|
||||
"Current password must be at least {minLength} characters.",
|
||||
newTooShort: "New password must be at least {minLength} characters.",
|
||||
newHasSpaces: "New password cannot contain spaces.",
|
||||
mismatch: "Passwords do not match.",
|
||||
samePassword:
|
||||
"New password must be different from your current password.",
|
||||
|
|
|
|||
|
|
@ -191,6 +191,10 @@ def prompt_new_password(verify_current: Callable[[str], bool], out: TextIO | Non
|
|||
out.write(f"Password must be at least {MIN_PASSWORD_LENGTH} characters. Try again.\n")
|
||||
out.flush()
|
||||
continue
|
||||
if any(ch.isspace() for ch in password):
|
||||
out.write("Password cannot contain spaces. Try again.\n")
|
||||
out.flush()
|
||||
continue
|
||||
if verify_current(password):
|
||||
out.write("New password must differ from the current password. Try again.\n")
|
||||
out.flush()
|
||||
|
|
@ -233,6 +237,8 @@ def validate_new_password(candidate: str, verify_current: Callable[[str], bool])
|
|||
current password), else None. Same policy as the interactive loop."""
|
||||
if len(candidate) < MIN_PASSWORD_LENGTH:
|
||||
return f"Password must be at least {MIN_PASSWORD_LENGTH} characters."
|
||||
if any(ch.isspace() for ch in candidate):
|
||||
return "Password cannot contain spaces."
|
||||
if verify_current(candidate):
|
||||
return "New password must differ from the current password."
|
||||
return None
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue