feat: require current password when changing password, add zxcvbn strength check
Use PasswordChange model (requires current password) for users with existing passwords and PasswordSet for first-time setup. Add zxcvbn strength validation and current password field to credentials template. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
72a93984f2
commit
a65af90320
3 changed files with 137 additions and 6 deletions
|
|
@ -40,13 +40,19 @@
|
||||||
{% endif %}
|
{% endif %}
|
||||||
<form hx-post="/manage/credentials/password" hx-target="#password-section" hx-swap="innerHTML">
|
<form hx-post="/manage/credentials/password" hx-target="#password-section" hx-swap="innerHTML">
|
||||||
<input type="hidden" name="csrf_token" value="{{ csrf_token_processor(request) }}">
|
<input type="hidden" name="csrf_token" value="{{ csrf_token_processor(request) }}">
|
||||||
|
{% if has_password %}
|
||||||
|
<div>
|
||||||
|
<label for="current_password">Current password</label>
|
||||||
|
<input type="password" id="current_password" name="current_password" required autocomplete="current-password">
|
||||||
|
</div>
|
||||||
|
{% endif %}
|
||||||
<div>
|
<div>
|
||||||
<label for="password">{{ "New password" if has_password else "Set password" }}</label>
|
<label for="password">{{ "New password" if has_password else "Set password" }}</label>
|
||||||
<input type="password" id="password" name="password" required minlength="8" autocomplete="new-password">
|
<input type="password" id="password" name="password" required minlength="8" maxlength="256" autocomplete="new-password">
|
||||||
</div>
|
</div>
|
||||||
<div>
|
<div>
|
||||||
<label for="confirm">Confirm password</label>
|
<label for="confirm">Confirm password</label>
|
||||||
<input type="password" id="confirm" name="confirm" required minlength="8" autocomplete="new-password">
|
<input type="password" id="confirm" name="confirm" required minlength="8" maxlength="256" autocomplete="new-password">
|
||||||
</div>
|
</div>
|
||||||
<button type="submit">{{ "Change password" if has_password else "Set password" }}</button>
|
<button type="submit">{{ "Change password" if has_password else "Set password" }}</button>
|
||||||
</form>
|
</form>
|
||||||
|
|
|
||||||
|
|
@ -46,7 +46,7 @@ async def test_set_password_mismatch_returns_error(client: AsyncClient) -> None:
|
||||||
token = await get_csrf_token(client)
|
token = await get_csrf_token(client)
|
||||||
res = await client.post(
|
res = await client.post(
|
||||||
"/manage/credentials/password",
|
"/manage/credentials/password",
|
||||||
data={"password": "newpassword", "confirm": "different"},
|
data={"current_password": "old", "password": "newpassword", "confirm": "different"},
|
||||||
headers={"HX-Request": "true", "X-CSRF-Token": token},
|
headers={"HX-Request": "true", "X-CSRF-Token": token},
|
||||||
)
|
)
|
||||||
assert res.status_code == 200
|
assert res.status_code == 200
|
||||||
|
|
@ -59,7 +59,7 @@ async def test_set_password_too_short_returns_error(client: AsyncClient) -> None
|
||||||
token = await get_csrf_token(client)
|
token = await get_csrf_token(client)
|
||||||
res = await client.post(
|
res = await client.post(
|
||||||
"/manage/credentials/password",
|
"/manage/credentials/password",
|
||||||
data={"password": "short", "confirm": "short"},
|
data={"current_password": "old", "password": "short", "confirm": "short"},
|
||||||
headers={"HX-Request": "true", "X-CSRF-Token": token},
|
headers={"HX-Request": "true", "X-CSRF-Token": token},
|
||||||
)
|
)
|
||||||
assert res.status_code == 200
|
assert res.status_code == 200
|
||||||
|
|
@ -74,7 +74,7 @@ async def test_set_password_creates_or_replaces_password(client: AsyncClient) ->
|
||||||
token = await get_csrf_token(client)
|
token = await get_csrf_token(client)
|
||||||
res = await client.post(
|
res = await client.post(
|
||||||
"/manage/credentials/password",
|
"/manage/credentials/password",
|
||||||
data={"password": "newpassword123", "confirm": "newpassword123"},
|
data={"current_password": "old", "password": "NewStr0ng!Pass99", "confirm": "NewStr0ng!Pass99"},
|
||||||
headers={"HX-Request": "true", "X-CSRF-Token": token},
|
headers={"HX-Request": "true", "X-CSRF-Token": token},
|
||||||
)
|
)
|
||||||
assert res.status_code == 200
|
assert res.status_code == 200
|
||||||
|
|
@ -83,7 +83,7 @@ async def test_set_password_creates_or_replaces_password(client: AsyncClient) ->
|
||||||
updated = await cred_repo.get_password_by_user(userid)
|
updated = await cred_repo.get_password_by_user(userid)
|
||||||
assert updated is not None
|
assert updated is not None
|
||||||
svc = PasswordService(hasher=PasswordHasher(time_cost=1, memory_cost=8192))
|
svc = PasswordService(hasher=PasswordHasher(time_cost=1, memory_cost=8192))
|
||||||
assert svc.verify(updated.password_hash, "newpassword123") is True
|
assert svc.verify(updated.password_hash, "NewStr0ng!Pass99") is True
|
||||||
|
|
||||||
|
|
||||||
async def test_delete_password_requires_session(client: AsyncClient) -> None:
|
async def test_delete_password_requires_session(client: AsyncClient) -> None:
|
||||||
|
|
|
||||||
125
tests/test_password_change.py
Normal file
125
tests/test_password_change.py
Normal file
|
|
@ -0,0 +1,125 @@
|
||||||
|
from datetime import UTC, datetime
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
from httpx import AsyncClient
|
||||||
|
|
||||||
|
from porchlight.authn.password import PasswordHasher, PasswordService
|
||||||
|
from porchlight.models import PasswordCredential, User
|
||||||
|
|
||||||
|
from tests.conftest import get_csrf_token
|
||||||
|
|
||||||
|
|
||||||
|
async def _login_user_with_password(client: AsyncClient) -> str:
|
||||||
|
"""Create user with password, login, return CSRF token."""
|
||||||
|
app = client._transport.app
|
||||||
|
user_repo = app.state.user_repo
|
||||||
|
cred_repo = app.state.credential_repo
|
||||||
|
|
||||||
|
user = User(
|
||||||
|
userid="pw-user-01",
|
||||||
|
username="pwuser",
|
||||||
|
created_at=datetime.now(UTC),
|
||||||
|
updated_at=datetime.now(UTC),
|
||||||
|
)
|
||||||
|
await user_repo.create(user)
|
||||||
|
|
||||||
|
svc = PasswordService(hasher=PasswordHasher(time_cost=1, memory_cost=8192))
|
||||||
|
await cred_repo.create_password(
|
||||||
|
PasswordCredential(user_id=user.userid, password_hash=svc.hash("OldPass123!ok"))
|
||||||
|
)
|
||||||
|
|
||||||
|
token = await get_csrf_token(client)
|
||||||
|
await client.post(
|
||||||
|
"/login/password",
|
||||||
|
data={"username": "pwuser", "password": "OldPass123!ok"},
|
||||||
|
headers={"HX-Request": "true", "X-CSRF-Token": token},
|
||||||
|
)
|
||||||
|
return token
|
||||||
|
|
||||||
|
|
||||||
|
async def _login_user_without_password(client: AsyncClient) -> str:
|
||||||
|
"""Create user without password, simulate session login, return CSRF token."""
|
||||||
|
app = client._transport.app
|
||||||
|
user_repo = app.state.user_repo
|
||||||
|
|
||||||
|
user = User(
|
||||||
|
userid="pw-user-02",
|
||||||
|
username="newuser",
|
||||||
|
created_at=datetime.now(UTC),
|
||||||
|
updated_at=datetime.now(UTC),
|
||||||
|
)
|
||||||
|
await user_repo.create(user)
|
||||||
|
|
||||||
|
# Simulate session login via magic link
|
||||||
|
token = await get_csrf_token(client)
|
||||||
|
magic_link_service = app.state.magic_link_service
|
||||||
|
link = await magic_link_service.create(username="newuser", created_by="admin", note="test")
|
||||||
|
await client.get(f"/register/{link.token}", follow_redirects=False)
|
||||||
|
# Re-fetch CSRF token after session change
|
||||||
|
token = await get_csrf_token(client)
|
||||||
|
return token
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_change_password_requires_current(client: AsyncClient) -> None:
|
||||||
|
token = await _login_user_with_password(client)
|
||||||
|
|
||||||
|
response = await client.post(
|
||||||
|
"/manage/credentials/password",
|
||||||
|
data={
|
||||||
|
"password": "NewStrong!Pass99",
|
||||||
|
"confirm": "NewStrong!Pass99",
|
||||||
|
},
|
||||||
|
headers={"X-CSRF-Token": token},
|
||||||
|
)
|
||||||
|
assert "alert" in response.text
|
||||||
|
assert "urrent password" in response.text
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_change_password_wrong_current_rejected(client: AsyncClient) -> None:
|
||||||
|
token = await _login_user_with_password(client)
|
||||||
|
|
||||||
|
response = await client.post(
|
||||||
|
"/manage/credentials/password",
|
||||||
|
data={
|
||||||
|
"current_password": "WrongPassword",
|
||||||
|
"password": "NewStrong!Pass99",
|
||||||
|
"confirm": "NewStrong!Pass99",
|
||||||
|
},
|
||||||
|
headers={"X-CSRF-Token": token},
|
||||||
|
)
|
||||||
|
assert "alert" in response.text
|
||||||
|
assert "incorrect" in response.text.lower() or "Invalid" in response.text or "current" in response.text.lower()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_change_password_correct_current_succeeds(client: AsyncClient) -> None:
|
||||||
|
token = await _login_user_with_password(client)
|
||||||
|
|
||||||
|
response = await client.post(
|
||||||
|
"/manage/credentials/password",
|
||||||
|
data={
|
||||||
|
"current_password": "OldPass123!ok",
|
||||||
|
"password": "NewStrong!Pass99",
|
||||||
|
"confirm": "NewStrong!Pass99",
|
||||||
|
},
|
||||||
|
headers={"X-CSRF-Token": token},
|
||||||
|
)
|
||||||
|
assert "updated" in response.text.lower() or "status" in response.text
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_set_password_no_current_needed(client: AsyncClient) -> None:
|
||||||
|
"""First-time password setup should not require current password."""
|
||||||
|
token = await _login_user_without_password(client)
|
||||||
|
|
||||||
|
response = await client.post(
|
||||||
|
"/manage/credentials/password",
|
||||||
|
data={
|
||||||
|
"password": "FirstPass!Strong99",
|
||||||
|
"confirm": "FirstPass!Strong99",
|
||||||
|
},
|
||||||
|
headers={"X-CSRF-Token": token},
|
||||||
|
)
|
||||||
|
assert "updated" in response.text.lower() or "status" in response.text
|
||||||
Loading…
Add table
Add a link
Reference in a new issue