diff --git a/CHANGELOG.md b/CHANGELOG.md index 69fc5dd76..d68e251b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ All notable changes to Bambuddy will be documented in this file. ## [0.2.4b1] - Unreleased ### Fixed +- **Setup: re-enabling auth could 422 on a password the form no longer needs** — after disabling authentication and re-enabling it (common when switching between local auth and LDAP, or recovering from a bad config), the setup form still sends `admin_password` in the body even though the backend route ignores it when an admin user already exists. The `SetupRequest` Pydantic schema enforced password complexity (uppercase + lowercase + digit + special char) unconditionally, so any existing password that predated the complexity rule — or a legitimate LDAP-mode placeholder — triggered `422 Value error, Password must contain at least one special character` before the route body could decide to ignore the field. Complexity validation has moved out of the schema and into the route body, scoped to the branch that actually creates a new local admin. Re-enabling auth with an existing admin (or any LDAP user) now accepts whatever the form sends; fresh first-time setup still rejects weak passwords with a clear 400. Two regression tests added in `test_auth_api.py`: weak password rejected at setup when creating the first admin, weak/placeholder password accepted when an admin already exists. - **Queue: batch (quantity>1) double-dispatched onto the same printer** — scheduling an ASAP print with `quantity > 1` could end up with two queue items in `'printing'` status for the same printer, surfaced in the logs as `BUG: Multiple queue items in 'printing' status for printer N`. The scheduler's in-memory `busy_printers` set was seeded empty each tick and only populated after `_start_print` succeeded in the current iteration, so on the next tick (30 s later) `_is_printer_idle()` read the printer's live MQTT state — which on H2D / P1 series lags several seconds behind the print command and still reported `IDLE` / `FINISH` — and dispatched the second batch item onto the already-running printer. `check_queue()` now queries `PrintQueueItem` for `status='printing'` rows and seeds `busy_printers` with their printer IDs before iterating pending items, so any printer with an outstanding dispatched job is excluded regardless of what MQTT currently reports. Regression covered in `test_phantom_print_hardening.py` (`TestBusyPrinterSeedingFromPrintingItems`): seeding query returns printers with `'printing'` rows only, returns empty when none exist, and end-to-end `check_queue()` does not call `_start_print` for a pending item whose printer already has a `'printing'` row even when `_is_printer_idle()` is forced `True`. - **Queue: active-item progress bar flashed 100% before dropping to 0%** — immediately after a queue item was dispatched, the per-item progress bar on the Queue page showed 100% (or whatever the prior print's final `mc_percent` was) for the few seconds between dispatch and the printer's MQTT state transitioning to `RUNNING`. Frontend `QueuePage.tsx` read `status.progress` directly from the printer's live MQTT snapshot, which carries over the last reported value from the previous print until the new one starts ticking. The progress bar, remaining time, ETA, and layer counter are now gated on `status.state` being `RUNNING` or `PAUSE`; in any other state (including `FINISH` from the prior print, `IDLE`, or `PREPARE` while heating) the bar renders at 0% with no stale ETA/layer values. diff --git a/backend/app/api/routes/auth.py b/backend/app/api/routes/auth.py index 8174b1b39..e48c6ec14 100644 --- a/backend/app/api/routes/auth.py +++ b/backend/app/api/routes/auth.py @@ -53,6 +53,7 @@ from backend.app.schemas.auth import ( TestSMTPRequest, TestSMTPResponse, UserResponse, + _validate_password_complexity, ) from backend.app.services.email_service import ( create_password_reset_link_email_from_template, @@ -228,6 +229,17 @@ async def setup_auth(request: SetupRequest, db: AsyncSession = Depends(get_db)): detail="Admin username and password are required when enabling authentication (no admin users exist)", ) + # Enforce password complexity only when actually creating a new admin. + # Schema-level validation was removed so that re-enabling auth with an + # existing admin (or LDAP) doesn't reject whatever placeholder the form sends. + try: + _validate_password_complexity(request.admin_password) + except ValueError as exc: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=str(exc), + ) + # Check if username already exists (shouldn't happen if no admin users exist, but check anyway) existing_user = await get_user_by_username(db, request.admin_username) if existing_user: diff --git a/backend/app/schemas/auth.py b/backend/app/schemas/auth.py index 6aa3b0bbd..ebd0e0d43 100644 --- a/backend/app/schemas/auth.py +++ b/backend/app/schemas/auth.py @@ -108,12 +108,11 @@ class SetupRequest(BaseModel): admin_username: str | None = Field(default=None, max_length=150) admin_password: str | None = Field(default=None, max_length=256) - @field_validator("admin_password") - @classmethod - def validate_admin_password(cls, v: str | None) -> str | None: - if v is not None: - _validate_password_complexity(v) - return v + # Password complexity is NOT validated at the schema layer. When re-enabling auth + # with an existing admin user (or when LDAP is the auth backend), the frontend + # still sends whatever is in the password field but the route ignores it. + # Enforcing complexity here would reject those legitimate flows. The route body + # applies the check only when a brand-new local admin is actually being created. class SetupResponse(BaseModel): diff --git a/backend/tests/integration/test_auth_api.py b/backend/tests/integration/test_auth_api.py index 04fab130c..3b38bca3b 100644 --- a/backend/tests/integration/test_auth_api.py +++ b/backend/tests/integration/test_auth_api.py @@ -70,6 +70,55 @@ class TestAuthSetupAPI: assert result["auth_enabled"] is True assert result["admin_created"] is True + @pytest.mark.asyncio + @pytest.mark.integration + async def test_setup_weak_password_rejected_when_creating_new_admin(self, async_client: AsyncClient): + """Complexity is enforced only when a new admin is being created.""" + response = await async_client.post( + "/api/v1/auth/setup", + json={ + "auth_enabled": True, + "admin_username": "weakpw_admin", + "admin_password": "NoSpecial1", + }, + ) + + assert response.status_code == 400 + assert "special character" in response.json()["detail"].lower() + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_setup_reenable_with_existing_admin_ignores_password(self, async_client: AsyncClient, db_session): + """Re-enabling auth when an admin already exists must not reject the placeholder + password the frontend still sends. Regression for the LDAP re-enable flow that + previously 422'd because the Pydantic schema enforced complexity unconditionally. + """ + from backend.app.core.auth import get_password_hash + from backend.app.models.user import User + + existing = User( + username="existing_admin", + password_hash=get_password_hash("DoesNotMatter1!"), + role="admin", + is_active=True, + ) + db_session.add(existing) + await db_session.commit() + + response = await async_client.post( + "/api/v1/auth/setup", + json={ + "auth_enabled": True, + "admin_username": "irrelevant", + "admin_password": "Ihk88LimT", + }, + ) + + assert response.status_code == 200 + result = response.json() + assert result["auth_enabled"] is True + assert result["admin_created"] is False + class TestAuthLoginAPI: """Integration tests for /api/v1/auth/login endpoint."""