Strengthen setup password validation
- Add backend Pydantic password complexity validation for setup - Update frontend setup page with password rule feedback and strength indicator - Add/adjust setup API tests for password validation - Document setup password requirements - Fix frontend test type annotation issue
This commit is contained in:
@@ -244,6 +244,7 @@ Backend: `http://127.0.0.1:8000` · Frontend (Vite proxy): `http://127.0.0.1:517
|
||||
### API login (dev)
|
||||
|
||||
The frontend SHA256-hashes the password before sending it to the API.
|
||||
The initial setup password must be at least 8 characters long and include one uppercase letter, one number, and one special character from `!@#$%^&*()`.
|
||||
The session cookie is named `bangui_session`.
|
||||
|
||||
```bash
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
Request, response, and domain models for the first-run configuration wizard.
|
||||
"""
|
||||
|
||||
from pydantic import BaseModel, ConfigDict, Field
|
||||
from pydantic import BaseModel, ConfigDict, Field, field_validator
|
||||
|
||||
|
||||
class SetupRequest(BaseModel):
|
||||
@@ -16,6 +16,22 @@ class SetupRequest(BaseModel):
|
||||
min_length=8,
|
||||
description="Master password that protects the BanGUI interface.",
|
||||
)
|
||||
|
||||
@field_validator("master_password")
|
||||
@classmethod
|
||||
def validate_master_password(cls, value: str) -> str:
|
||||
if len(value) < 8:
|
||||
raise ValueError("Password must be at least 8 characters long.")
|
||||
if not any(char.isupper() for char in value):
|
||||
raise ValueError("Password must include at least one uppercase letter.")
|
||||
if not any(char.isdigit() for char in value):
|
||||
raise ValueError("Password must include at least one number.")
|
||||
if not any(char in "!@#$%^&*()" for char in value):
|
||||
raise ValueError(
|
||||
"Password must include at least one special character (!@#$%^&*())."
|
||||
)
|
||||
return value
|
||||
|
||||
database_path: str = Field(
|
||||
default="bangui.db",
|
||||
description="Filesystem path to the BanGUI SQLite application database.",
|
||||
|
||||
@@ -298,7 +298,7 @@ async def test_startup_overrides_settings_from_persisted_setup(tmp_path: Path) -
|
||||
await init_db(db)
|
||||
await setup_service.run_setup(
|
||||
db,
|
||||
master_password="supersecret123",
|
||||
master_password="Supersecret1!",
|
||||
database_path=runtime_db_path,
|
||||
fail2ban_socket="/tmp/persisted.sock",
|
||||
timezone="Europe/Berlin",
|
||||
|
||||
@@ -19,7 +19,7 @@ from app.services import setup_service
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
_SETUP_PAYLOAD: dict[str, object] = {
|
||||
"master_password": "supersecret123",
|
||||
"master_password": "Supersecret1!",
|
||||
"database_path": "bangui.db",
|
||||
"fail2ban_socket": "/var/run/fail2ban/fail2ban.sock",
|
||||
"timezone": "UTC",
|
||||
@@ -78,7 +78,7 @@ class TestGetSetupStatus:
|
||||
await client.post(
|
||||
"/api/setup",
|
||||
json={
|
||||
"master_password": "supersecret123",
|
||||
"master_password": "Supersecret1!",
|
||||
"database_path": "bangui.db",
|
||||
"fail2ban_socket": "/var/run/fail2ban/fail2ban.sock",
|
||||
"timezone": "UTC",
|
||||
@@ -98,7 +98,7 @@ class TestPostSetup:
|
||||
response = await client.post(
|
||||
"/api/setup",
|
||||
json={
|
||||
"master_password": "supersecret123",
|
||||
"master_password": "Supersecret1!",
|
||||
"database_path": "bangui.db",
|
||||
"fail2ban_socket": "/var/run/fail2ban/fail2ban.sock",
|
||||
"timezone": "UTC",
|
||||
@@ -117,10 +117,48 @@ class TestPostSetup:
|
||||
)
|
||||
assert response.status_code == 422
|
||||
|
||||
async def test_rejects_missing_uppercase_password(self, client: AsyncClient) -> None:
|
||||
"""Setup endpoint rejects passwords missing an uppercase character."""
|
||||
response = await client.post(
|
||||
"/api/setup",
|
||||
json={"master_password": "lowercase1!"},
|
||||
)
|
||||
assert response.status_code == 422
|
||||
assert any(
|
||||
"uppercase" in error["msg"].lower()
|
||||
for error in response.json()["detail"]
|
||||
)
|
||||
|
||||
async def test_rejects_missing_number_password(self, client: AsyncClient) -> None:
|
||||
"""Setup endpoint rejects passwords missing a numeric character."""
|
||||
response = await client.post(
|
||||
"/api/setup",
|
||||
json={"master_password": "NoNumbers!"},
|
||||
)
|
||||
assert response.status_code == 422
|
||||
assert any(
|
||||
"number" in error["msg"].lower()
|
||||
for error in response.json()["detail"]
|
||||
)
|
||||
|
||||
async def test_rejects_missing_special_character_password(
|
||||
self, client: AsyncClient
|
||||
) -> None:
|
||||
"""Setup endpoint rejects passwords missing a required special character."""
|
||||
response = await client.post(
|
||||
"/api/setup",
|
||||
json={"master_password": "NoSpecial1"},
|
||||
)
|
||||
assert response.status_code == 422
|
||||
assert any(
|
||||
"special character" in error["msg"].lower()
|
||||
for error in response.json()["detail"]
|
||||
)
|
||||
|
||||
async def test_rejects_second_call(self, client: AsyncClient) -> None:
|
||||
"""Setup endpoint returns 409 if setup has already been completed."""
|
||||
payload = {
|
||||
"master_password": "supersecret123",
|
||||
"master_password": "Supersecret1!",
|
||||
"database_path": "bangui.db",
|
||||
"fail2ban_socket": "/var/run/fail2ban/fail2ban.sock",
|
||||
"timezone": "UTC",
|
||||
@@ -138,7 +176,7 @@ class TestPostSetup:
|
||||
"""Setup endpoint uses defaults when optional fields are omitted."""
|
||||
response = await client.post(
|
||||
"/api/setup",
|
||||
json={"master_password": "supersecret123"},
|
||||
json={"master_password": "Supersecret1!"},
|
||||
)
|
||||
assert response.status_code == 201
|
||||
|
||||
@@ -150,7 +188,7 @@ class TestPostSetupRuntimeState:
|
||||
"""App state should reflect setup settings immediately after setup."""
|
||||
app, client = app_and_client
|
||||
payload = {
|
||||
"master_password": "supersecret123",
|
||||
"master_password": "Supersecret1!",
|
||||
"database_path": "bangui.db",
|
||||
"fail2ban_socket": "/tmp/persisted.sock",
|
||||
"timezone": "Europe/Berlin",
|
||||
@@ -193,7 +231,7 @@ class TestSetupRedirectMiddleware:
|
||||
"""Protected endpoints are reachable (no redirect) after setup."""
|
||||
await client.post(
|
||||
"/api/setup",
|
||||
json={"master_password": "supersecret123"},
|
||||
json={"master_password": "Supersecret1!"},
|
||||
)
|
||||
# /api/auth/login should now be reachable (returns 405 GET not allowed,
|
||||
# not a setup redirect)
|
||||
@@ -220,7 +258,7 @@ class TestGetTimezone:
|
||||
await client.post(
|
||||
"/api/setup",
|
||||
json={
|
||||
"master_password": "supersecret123",
|
||||
"master_password": "Supersecret1!",
|
||||
"timezone": "Europe/Berlin",
|
||||
},
|
||||
)
|
||||
@@ -413,7 +451,7 @@ class TestLifespanSetupCache:
|
||||
await init_db(db)
|
||||
await setup_service.run_setup(
|
||||
db,
|
||||
master_password="supersecret123",
|
||||
master_password="Supersecret1!",
|
||||
database_path=settings.database_path,
|
||||
fail2ban_socket=settings.fail2ban_socket,
|
||||
timezone=settings.timezone,
|
||||
|
||||
@@ -6,7 +6,7 @@
|
||||
* All fields use Fluent UI v9 components and inline validation.
|
||||
*/
|
||||
|
||||
import { useEffect, useState } from "react";
|
||||
import { useEffect, useState, type ChangeEvent, type FormEvent } from "react";
|
||||
import {
|
||||
Button,
|
||||
Field,
|
||||
@@ -19,7 +19,6 @@ import {
|
||||
tokens,
|
||||
} from "@fluentui/react-components";
|
||||
import { useNavigate } from "react-router-dom";
|
||||
import type { ChangeEvent, FormEvent } from "react";
|
||||
import { useSetup } from "../hooks/useSetup";
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -63,6 +62,40 @@ const useStyles = makeStyles({
|
||||
error: {
|
||||
marginBottom: tokens.spacingVerticalM,
|
||||
},
|
||||
passwordStrength: {
|
||||
marginTop: tokens.spacingVerticalS,
|
||||
display: "grid",
|
||||
gap: tokens.spacingVerticalS,
|
||||
},
|
||||
passwordStrengthBar: {
|
||||
display: "grid",
|
||||
gridTemplateColumns: "repeat(4, 1fr)",
|
||||
gap: tokens.spacingHorizontalS,
|
||||
width: "100%",
|
||||
},
|
||||
passwordStrengthSegment: {
|
||||
height: "8px",
|
||||
borderRadius: tokens.borderRadiusSmall,
|
||||
backgroundColor: tokens.colorNeutralStroke2,
|
||||
},
|
||||
passwordStrengthSegmentActive: {
|
||||
backgroundColor: tokens.colorPaletteGreenBorder1,
|
||||
},
|
||||
passwordRuleList: {
|
||||
margin: 0,
|
||||
paddingLeft: tokens.spacingHorizontalL,
|
||||
color: tokens.colorNeutralForeground2,
|
||||
fontSize: "0.875rem",
|
||||
},
|
||||
passwordRuleItem: {
|
||||
marginBottom: tokens.spacingVerticalXS,
|
||||
},
|
||||
passwordRuleItemPassed: {
|
||||
color: tokens.colorPaletteGreenForeground1,
|
||||
},
|
||||
passwordRuleItemFailed: {
|
||||
color: tokens.colorPaletteRedForeground1,
|
||||
},
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -87,6 +120,46 @@ const DEFAULT_VALUES: FormValues = {
|
||||
sessionDurationMinutes: "60",
|
||||
};
|
||||
|
||||
type ValidationErrors = Partial<Record<keyof FormValues, string>>;
|
||||
|
||||
type PasswordRuleId = "length" | "uppercase" | "number" | "special";
|
||||
|
||||
interface PasswordRule {
|
||||
id: PasswordRuleId;
|
||||
label: string;
|
||||
test: (password: string) => boolean;
|
||||
}
|
||||
|
||||
const PASSWORD_RULES: PasswordRule[] = [
|
||||
{
|
||||
id: "length",
|
||||
label: "At least 8 characters",
|
||||
test: (password: string) => password.length >= 8,
|
||||
},
|
||||
{
|
||||
id: "uppercase",
|
||||
label: "At least one uppercase letter",
|
||||
test: (password: string) => /[A-Z]/.test(password),
|
||||
},
|
||||
{
|
||||
id: "number",
|
||||
label: "At least one number",
|
||||
test: (password: string) => /\d/.test(password),
|
||||
},
|
||||
{
|
||||
id: "special",
|
||||
label: "At least one special character (!@#$%^&*())",
|
||||
test: (password: string) => /[!@#$%^&*()]/.test(password),
|
||||
},
|
||||
];
|
||||
|
||||
function getPasswordRuleStatus(password: string) {
|
||||
return PASSWORD_RULES.map((rule) => ({
|
||||
...rule,
|
||||
satisfied: rule.test(password),
|
||||
}));
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Component
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -101,7 +174,9 @@ export function SetupPage(): React.JSX.Element {
|
||||
|
||||
const { status, loading, error, submit, submitting, submitError } = useSetup();
|
||||
const [values, setValues] = useState<FormValues>(DEFAULT_VALUES);
|
||||
const [errors, setErrors] = useState<Partial<Record<keyof FormValues, string>>>({});
|
||||
const [errors, setErrors] = useState<ValidationErrors>({});
|
||||
const passwordRules = getPasswordRuleStatus(values.masterPassword);
|
||||
const passwordStrength = passwordRules.filter((rule) => rule.satisfied).length;
|
||||
const apiError = error ?? submitError;
|
||||
|
||||
// Redirect to /login if setup has already been completed.
|
||||
@@ -125,11 +200,15 @@ export function SetupPage(): React.JSX.Element {
|
||||
}
|
||||
|
||||
function validate(): boolean {
|
||||
const next: Partial<Record<keyof FormValues, string>> = {};
|
||||
const next: ValidationErrors = {};
|
||||
const unmetPasswordRules = passwordRules.filter((rule) => !rule.satisfied);
|
||||
|
||||
if (values.masterPassword.length < 8) {
|
||||
next.masterPassword = "Password must be at least 8 characters.";
|
||||
if (!values.masterPassword.trim()) {
|
||||
next.masterPassword = "Password is required.";
|
||||
} else if (unmetPasswordRules.length > 0) {
|
||||
next.masterPassword = "Password must meet all complexity requirements.";
|
||||
}
|
||||
|
||||
if (values.masterPassword !== values.confirmPassword) {
|
||||
next.confirmPassword = "Passwords do not match.";
|
||||
}
|
||||
@@ -219,8 +298,34 @@ export function SetupPage(): React.JSX.Element {
|
||||
<Field
|
||||
label="Master Password"
|
||||
required
|
||||
validationMessage={errors.masterPassword}
|
||||
validationState={errors.masterPassword ? "error" : "none"}
|
||||
validationMessage={
|
||||
errors.masterPassword ??
|
||||
(passwordRules.some((rule) => !rule.satisfied)
|
||||
? {
|
||||
children: (
|
||||
<ul className={styles.passwordRuleList}>
|
||||
{passwordRules.map((rule) => (
|
||||
<li
|
||||
key={rule.id}
|
||||
className={
|
||||
rule.satisfied
|
||||
? styles.passwordRuleItemPassed
|
||||
: styles.passwordRuleItemFailed
|
||||
}
|
||||
>
|
||||
{rule.label}
|
||||
</li>
|
||||
))}
|
||||
</ul>
|
||||
),
|
||||
}
|
||||
: undefined)
|
||||
}
|
||||
validationState={
|
||||
errors.masterPassword || passwordRules.some((rule) => !rule.satisfied)
|
||||
? "error"
|
||||
: "none"
|
||||
}
|
||||
>
|
||||
<Input
|
||||
type="password"
|
||||
@@ -228,6 +333,23 @@ export function SetupPage(): React.JSX.Element {
|
||||
onChange={handleChange("masterPassword")}
|
||||
autoComplete="new-password"
|
||||
/>
|
||||
<div className={styles.passwordStrength} aria-live="polite">
|
||||
<div className={styles.passwordStrengthBar}>
|
||||
{passwordRules.map((rule) => (
|
||||
<span
|
||||
key={rule.id}
|
||||
className={
|
||||
rule.satisfied
|
||||
? `${styles.passwordStrengthSegment} ${styles.passwordStrengthSegmentActive}`
|
||||
: styles.passwordStrengthSegment
|
||||
}
|
||||
/>
|
||||
))}
|
||||
</div>
|
||||
<Text size={200}>
|
||||
{passwordStrength} of {passwordRules.length} rules satisfied
|
||||
</Text>
|
||||
</div>
|
||||
</Field>
|
||||
|
||||
<Field
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { render, screen, waitFor } from "@testing-library/react";
|
||||
import userEvent from "@testing-library/user-event";
|
||||
import { MemoryRouter, Routes, Route } from "react-router-dom";
|
||||
import { FluentProvider, webLightTheme } from "@fluentui/react-components";
|
||||
import { SetupPage } from "../SetupPage";
|
||||
@@ -10,9 +11,10 @@ vi.mock("../../api/setup", () => ({
|
||||
submitSetup: vi.fn(),
|
||||
}));
|
||||
|
||||
import { getSetupStatus } from "../../api/setup";
|
||||
import { getSetupStatus, submitSetup } from "../../api/setup";
|
||||
|
||||
const mockedGetSetupStatus = vi.mocked(getSetupStatus);
|
||||
const mockedSubmitSetup = vi.mocked(submitSetup);
|
||||
|
||||
function renderPage() {
|
||||
return render(
|
||||
@@ -59,6 +61,45 @@ describe("SetupPage", () => {
|
||||
expect(screen.queryByRole("progressbar")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("displays password complexity feedback while the user types", async () => {
|
||||
mockedGetSetupStatus.mockResolvedValue({ completed: false });
|
||||
renderPage();
|
||||
|
||||
await waitFor(() => {
|
||||
expect(
|
||||
screen.getByRole("heading", { name: /bangui setup/i }),
|
||||
).toBeInTheDocument();
|
||||
});
|
||||
|
||||
const user = userEvent.setup();
|
||||
const passwordInput = screen.getByLabelText(/master password/i);
|
||||
await user.type(passwordInput, "Short1");
|
||||
|
||||
expect(screen.getByText(/2 of 4 rules satisfied/i)).toBeInTheDocument();
|
||||
expect(screen.getByText(/at least 8 characters/i)).toBeInTheDocument();
|
||||
expect(screen.getByText(/at least one uppercase letter/i)).toBeInTheDocument();
|
||||
expect(screen.getByText(/at least one special character/i)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("does not submit the form when the password is too weak", async () => {
|
||||
mockedGetSetupStatus.mockResolvedValue({ completed: false });
|
||||
mockedSubmitSetup.mockResolvedValue({ message: "Setup completed successfully. Please log in." });
|
||||
renderPage();
|
||||
|
||||
await waitFor(() => {
|
||||
expect(
|
||||
screen.getByRole("heading", { name: /bangui setup/i }),
|
||||
).toBeInTheDocument();
|
||||
});
|
||||
|
||||
const user = userEvent.setup();
|
||||
await user.type(screen.getByLabelText(/master password/i), "Short1");
|
||||
await user.type(screen.getByLabelText(/confirm password/i), "Short1");
|
||||
await user.click(screen.getByRole("button", { name: /complete setup/i }));
|
||||
|
||||
expect(mockedSubmitSetup).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("redirects to /login when setup is already complete", async () => {
|
||||
mockedGetSetupStatus.mockResolvedValue({ completed: true });
|
||||
renderPage();
|
||||
|
||||
@@ -1,9 +1,10 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { areHistoryQueriesEqual } from "../queryUtils";
|
||||
import type { HistoryQuery } from "../../types/history";
|
||||
|
||||
describe("areHistoryQueriesEqual", () => {
|
||||
it("returns true for identical history queries", () => {
|
||||
const a = {
|
||||
const a: HistoryQuery = {
|
||||
range: "7d",
|
||||
origin: "blocklist",
|
||||
jail: "ssh",
|
||||
@@ -19,7 +20,7 @@ describe("areHistoryQueriesEqual", () => {
|
||||
});
|
||||
|
||||
it("returns false when a single query field differs", () => {
|
||||
const base = {
|
||||
const base: HistoryQuery = {
|
||||
range: "7d",
|
||||
origin: "all",
|
||||
jail: "ssh",
|
||||
|
||||
Reference in New Issue
Block a user