mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-10-22 03:49:18 +02:00
feat(auth): admin-configurable session lifetime ceiling (#1706)
The 24h session cap from the M-2 audit finding was hard-coded, so the
"Remember Me" checkbox could only control storage location, never
duration. Add session_max_hours setting (default 24, max 720) honoured
at all four token-issuance sites: plain login, 2FA TOTP/email, 2FA
backup, OIDC.
- backend/app/core/auth.py: SESSION_MAX_HOURS_HARD_CEILING + resolver
that clamps to [1h, 720h] and falls back to 24h on missing/blank/
unparseable. DB errors propagate — the login transaction must abort
on a broken DB rather than silently extend or shrink the lifetime.
- backend/app/api/routes/auth.py, mfa.py: all four sites read the
resolved value instead of ACCESS_TOKEN_EXPIRE_MINUTES directly.
- backend/app/schemas/settings.py, routes/settings.py: schema field
with ge=1 le=720 + int coercion in _build_settings_response.
- frontend/src/pages/SettingsPage.tsx: half-width card at top of
Settings -> Users left column with 24h/7d/30d presets, custom input,
and a yellow warning when value > 24h.
- frontend/src/i18n/locales/*.ts: 8 new keys per locale, real
translations in all 11 (en/de/es/fr/it/ja/ko/pt-BR/tr/zh-CN/zh-TW).
- backend/tests/integration/test_session_policy.py: 15 tests across
resolver clamping, login JWT exp end-to-end, settings API round-trip.
Already-issued tokens keep their original expiry; the new setting only
affects future logins.
This commit is contained in:
@@ -15,7 +15,6 @@ from sqlalchemy.orm import selectinload
|
||||
|
||||
from backend.app.api.routes.settings import get_external_login_url
|
||||
from backend.app.core.auth import (
|
||||
ACCESS_TOKEN_EXPIRE_MINUTES,
|
||||
ALGORITHM,
|
||||
SECRET_KEY,
|
||||
Permission,
|
||||
@@ -31,6 +30,7 @@ from backend.app.core.auth import (
|
||||
get_user_by_email,
|
||||
get_user_by_username,
|
||||
is_jti_revoked,
|
||||
resolve_session_max_minutes,
|
||||
revoke_jti,
|
||||
security,
|
||||
)
|
||||
@@ -495,8 +495,9 @@ async def login(raw_request: Request, request: LoginRequest, response: Response,
|
||||
two_fa_methods=methods,
|
||||
)
|
||||
|
||||
# No 2FA — issue full token immediately
|
||||
access_token_expires = timedelta(minutes=ACCESS_TOKEN_EXPIRE_MINUTES)
|
||||
# No 2FA — issue full token immediately. Session lifetime honours the
|
||||
# admin-configurable ceiling (#1706); resolver clamps to [1h, 720h].
|
||||
access_token_expires = timedelta(minutes=await resolve_session_max_minutes(db))
|
||||
access_token = create_access_token(data={"sub": user.username}, expires_delta=access_token_expires)
|
||||
|
||||
return LoginResponse(
|
||||
|
||||
@@ -41,13 +41,13 @@ from sqlalchemy.orm import selectinload, undefer
|
||||
from backend.app.api.routes._oidc_helpers import assert_safe_public_https_url
|
||||
from backend.app.api.routes.settings import get_setting, set_setting
|
||||
from backend.app.core.auth import (
|
||||
ACCESS_TOKEN_EXPIRE_MINUTES,
|
||||
RequirePermissionIfAuthEnabled,
|
||||
create_access_token,
|
||||
get_current_active_user,
|
||||
get_user_by_email,
|
||||
get_user_by_username,
|
||||
is_auth_enabled,
|
||||
resolve_session_max_minutes,
|
||||
verify_password,
|
||||
)
|
||||
from backend.app.core.database import get_db
|
||||
@@ -1242,7 +1242,7 @@ async def verify_2fa(
|
||||
|
||||
access_token = create_access_token(
|
||||
data={"sub": user.username},
|
||||
expires_delta=timedelta(minutes=ACCESS_TOKEN_EXPIRE_MINUTES),
|
||||
expires_delta=timedelta(minutes=await resolve_session_max_minutes(db)),
|
||||
)
|
||||
result = await db.execute(select(User).where(User.id == user.id).options(selectinload(User.groups)))
|
||||
user = result.scalar_one()
|
||||
@@ -1258,7 +1258,7 @@ async def verify_2fa(
|
||||
|
||||
access_token = create_access_token(
|
||||
data={"sub": user.username},
|
||||
expires_delta=timedelta(minutes=ACCESS_TOKEN_EXPIRE_MINUTES),
|
||||
expires_delta=timedelta(minutes=await resolve_session_max_minutes(db)),
|
||||
)
|
||||
|
||||
# Reload with groups for permission calculation
|
||||
@@ -2146,7 +2146,7 @@ async def oidc_exchange(
|
||||
|
||||
access_token = create_access_token(
|
||||
data={"sub": user.username},
|
||||
expires_delta=timedelta(minutes=ACCESS_TOKEN_EXPIRE_MINUTES),
|
||||
expires_delta=timedelta(minutes=await resolve_session_max_minutes(db)),
|
||||
)
|
||||
|
||||
return LoginResponse(
|
||||
|
||||
@@ -159,6 +159,7 @@ async def _build_settings_response(db: AsyncSession, is_api_key: bool = False) -
|
||||
"stagger_group_size",
|
||||
"stagger_interval_minutes",
|
||||
"forecast_global_lead_time_days",
|
||||
"session_max_hours",
|
||||
]:
|
||||
settings_dict[setting.key] = int(setting.value)
|
||||
elif setting.key == "default_printer_id":
|
||||
|
||||
@@ -421,10 +421,42 @@ def _get_jwt_secret() -> str:
|
||||
SECRET_KEY = _get_jwt_secret()
|
||||
ALGORITHM = "HS256"
|
||||
ACCESS_TOKEN_EXPIRE_MINUTES = 60 * 24 # 24 hours (M-2: reduced from 7 days)
|
||||
# Hard ceiling for the admin-configurable session policy (#1706). 30 days
|
||||
# matches the Pydantic le=720 on AppSettings.session_max_hours; defense in
|
||||
# depth so a tampered settings row can't request an absurd lifetime.
|
||||
SESSION_MAX_HOURS_HARD_CEILING = 720
|
||||
|
||||
# HTTP Bearer token
|
||||
security = HTTPBearer(auto_error=False)
|
||||
|
||||
|
||||
async def resolve_session_max_minutes(db: AsyncSession) -> int:
|
||||
"""Return the session-lifetime ceiling (minutes) honoured by login routes.
|
||||
|
||||
Reads ``session_max_hours`` from the settings table (#1706), clamps to
|
||||
[1h, 720h], and falls back to the audit-default 24h if the row is
|
||||
missing, blank, or unparseable.
|
||||
|
||||
DB errors are NOT caught here — login is already in a DB transaction and
|
||||
a broken DB must abort the login rather than silently extend or shrink
|
||||
the session lifetime.
|
||||
"""
|
||||
default_minutes = ACCESS_TOKEN_EXPIRE_MINUTES
|
||||
result = await db.execute(select(Settings).where(Settings.key == "session_max_hours"))
|
||||
row = result.scalar_one_or_none()
|
||||
if row is None or not row.value:
|
||||
return default_minutes
|
||||
try:
|
||||
hours = int(row.value)
|
||||
except (TypeError, ValueError):
|
||||
return default_minutes
|
||||
if hours < 1:
|
||||
return default_minutes
|
||||
if hours > SESSION_MAX_HOURS_HARD_CEILING:
|
||||
hours = SESSION_MAX_HOURS_HARD_CEILING
|
||||
return hours * 60
|
||||
|
||||
|
||||
# --- Slicer download tokens ---
|
||||
# Short-lived, single-use tokens for slicer protocol handlers that can't send
|
||||
# auth headers. Stored in AuthEphemeralToken (token_type=TokenType.SLICER_DOWNLOAD)
|
||||
@@ -649,7 +681,9 @@ def _is_token_fresh(iat: int | float | None, user: User) -> bool:
|
||||
Used to invalidate all sessions after a password reset/change (M-R7-B).
|
||||
All tokens without an iat claim are unconditionally rejected — every token
|
||||
issued by this server carries iat, so absence means the token is forged or
|
||||
from a pre-iat code path whose max TTL (24 h) has long since expired.
|
||||
from a pre-iat code path whose max TTL at the time (24 h) has long since
|
||||
expired. The post-#1706 admin-set ceiling does not relax this — an iat-less
|
||||
token still cannot have been issued by current code.
|
||||
"""
|
||||
if iat is None:
|
||||
return False
|
||||
|
||||
@@ -240,6 +240,20 @@ class AppSettings(BaseModel):
|
||||
description="Low stock threshold percentage (%) for inventory filtering and display",
|
||||
)
|
||||
|
||||
# Session policy (#1706) — admin-set ceiling for user session lifetime.
|
||||
# Default 24h preserves the M-2 audit reduction from 7 days. Max 720h
|
||||
# (30 days) bounds blast radius if an admin chooses a long session.
|
||||
session_max_hours: int = Field(
|
||||
default=24,
|
||||
ge=1,
|
||||
le=720,
|
||||
description=(
|
||||
"Maximum session lifetime in hours for user logins (default 24, max 720). "
|
||||
"Applies to new logins only; already-issued tokens keep their original expiry. "
|
||||
"Longer sessions reduce automatic logout protection."
|
||||
),
|
||||
)
|
||||
|
||||
# User email notifications (requires Advanced Authentication)
|
||||
user_notifications_enabled: bool = Field(
|
||||
default=True,
|
||||
@@ -414,6 +428,7 @@ class AppSettingsUpdate(BaseModel):
|
||||
prometheus_enabled: bool | None = None
|
||||
prometheus_token: str | None = None
|
||||
low_stock_threshold: float | None = Field(default=None, ge=0.1, le=99.9)
|
||||
session_max_hours: int | None = Field(default=None, ge=1, le=720)
|
||||
user_notifications_enabled: bool | None = None
|
||||
default_bed_levelling: bool | None = None
|
||||
default_flow_cali: bool | None = None
|
||||
|
||||
@@ -0,0 +1,229 @@
|
||||
"""Integration tests for the admin-set session-lifetime ceiling (#1706).
|
||||
|
||||
Covers the four token-issuance sites that read ``session_max_hours``:
|
||||
plain login, 2FA backup-code login, 2FA TOTP/email login, OIDC login.
|
||||
Only the first is exercised end-to-end via ``async_client``; the helper
|
||||
``resolve_session_max_minutes`` itself is unit-tested below so the MFA
|
||||
and OIDC paths inherit the same clamping behaviour by construction.
|
||||
"""
|
||||
|
||||
import time
|
||||
|
||||
import jwt
|
||||
import pytest
|
||||
from httpx import AsyncClient
|
||||
from sqlalchemy import select
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from backend.app.core.auth import (
|
||||
ACCESS_TOKEN_EXPIRE_MINUTES,
|
||||
ALGORITHM,
|
||||
SECRET_KEY,
|
||||
SESSION_MAX_HOURS_HARD_CEILING,
|
||||
resolve_session_max_minutes,
|
||||
)
|
||||
from backend.app.models.settings import Settings
|
||||
|
||||
|
||||
async def _set_session_max_hours(db: AsyncSession, value: str | None) -> None:
|
||||
"""Upsert the session_max_hours setting row (value=None deletes it)."""
|
||||
result = await db.execute(select(Settings).where(Settings.key == "session_max_hours"))
|
||||
existing = result.scalar_one_or_none()
|
||||
if value is None:
|
||||
if existing is not None:
|
||||
await db.delete(existing)
|
||||
await db.commit()
|
||||
return
|
||||
if existing is None:
|
||||
db.add(Settings(key="session_max_hours", value=value))
|
||||
else:
|
||||
existing.value = value
|
||||
await db.commit()
|
||||
|
||||
|
||||
class TestResolveSessionMaxMinutes:
|
||||
"""Unit-style tests for the clamping resolver."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_missing_row_returns_24h_default(self, db_session: AsyncSession):
|
||||
await _set_session_max_hours(db_session, None)
|
||||
assert await resolve_session_max_minutes(db_session) == ACCESS_TOKEN_EXPIRE_MINUTES
|
||||
assert ACCESS_TOKEN_EXPIRE_MINUTES == 60 * 24
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_empty_string_returns_24h_default(self, db_session: AsyncSession):
|
||||
await _set_session_max_hours(db_session, "")
|
||||
assert await resolve_session_max_minutes(db_session) == ACCESS_TOKEN_EXPIRE_MINUTES
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_unparseable_value_returns_24h_default(self, db_session: AsyncSession):
|
||||
await _set_session_max_hours(db_session, "not-a-number")
|
||||
assert await resolve_session_max_minutes(db_session) == ACCESS_TOKEN_EXPIRE_MINUTES
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_zero_or_negative_returns_24h_default(self, db_session: AsyncSession):
|
||||
await _set_session_max_hours(db_session, "0")
|
||||
assert await resolve_session_max_minutes(db_session) == ACCESS_TOKEN_EXPIRE_MINUTES
|
||||
await _set_session_max_hours(db_session, "-5")
|
||||
assert await resolve_session_max_minutes(db_session) == ACCESS_TOKEN_EXPIRE_MINUTES
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_one_hour_minimum(self, db_session: AsyncSession):
|
||||
await _set_session_max_hours(db_session, "1")
|
||||
assert await resolve_session_max_minutes(db_session) == 60
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_seven_days_passes_through(self, db_session: AsyncSession):
|
||||
await _set_session_max_hours(db_session, "168")
|
||||
assert await resolve_session_max_minutes(db_session) == 168 * 60
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_thirty_days_passes_through(self, db_session: AsyncSession):
|
||||
await _set_session_max_hours(db_session, str(SESSION_MAX_HOURS_HARD_CEILING))
|
||||
assert await resolve_session_max_minutes(db_session) == SESSION_MAX_HOURS_HARD_CEILING * 60
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_above_ceiling_is_clamped_to_30_days(self, db_session: AsyncSession):
|
||||
"""Defense-in-depth: a tampered settings row above 720h must be clamped."""
|
||||
await _set_session_max_hours(db_session, "99999")
|
||||
assert await resolve_session_max_minutes(db_session) == SESSION_MAX_HOURS_HARD_CEILING * 60
|
||||
|
||||
|
||||
class TestLoginRespectsSessionPolicy:
|
||||
"""The /auth/login route must honour the resolved ceiling."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_login_uses_default_24h_when_unset(self, async_client: AsyncClient, db_session: AsyncSession):
|
||||
await async_client.post(
|
||||
"/api/v1/auth/setup",
|
||||
json={
|
||||
"auth_enabled": True,
|
||||
"admin_username": "sessiontest1",
|
||||
"admin_password": "SessionPass1!",
|
||||
},
|
||||
)
|
||||
await _set_session_max_hours(db_session, None)
|
||||
|
||||
before = int(time.time())
|
||||
response = await async_client.post(
|
||||
"/api/v1/auth/login",
|
||||
json={"username": "sessiontest1", "password": "SessionPass1!"},
|
||||
)
|
||||
after = int(time.time())
|
||||
|
||||
assert response.status_code == 200
|
||||
token = response.json()["access_token"]
|
||||
decoded = jwt.decode(token, SECRET_KEY, algorithms=[ALGORITHM])
|
||||
# exp should be ~24h ahead. Allow generous bounds for clock drift.
|
||||
expected_min = before + 24 * 3600 - 60
|
||||
expected_max = after + 24 * 3600 + 60
|
||||
assert expected_min <= decoded["exp"] <= expected_max
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_login_uses_configured_7d_ceiling(self, async_client: AsyncClient, db_session: AsyncSession):
|
||||
await async_client.post(
|
||||
"/api/v1/auth/setup",
|
||||
json={
|
||||
"auth_enabled": True,
|
||||
"admin_username": "sessiontest2",
|
||||
"admin_password": "SessionPass2!",
|
||||
},
|
||||
)
|
||||
await _set_session_max_hours(db_session, "168") # 7 days
|
||||
|
||||
before = int(time.time())
|
||||
response = await async_client.post(
|
||||
"/api/v1/auth/login",
|
||||
json={"username": "sessiontest2", "password": "SessionPass2!"},
|
||||
)
|
||||
after = int(time.time())
|
||||
|
||||
assert response.status_code == 200
|
||||
token = response.json()["access_token"]
|
||||
decoded = jwt.decode(token, SECRET_KEY, algorithms=[ALGORITHM])
|
||||
expected_min = before + 168 * 3600 - 60
|
||||
expected_max = after + 168 * 3600 + 60
|
||||
assert expected_min <= decoded["exp"] <= expected_max
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_login_clamps_above_ceiling(self, async_client: AsyncClient, db_session: AsyncSession):
|
||||
"""A settings row above the 720h ceiling must be clamped at login time."""
|
||||
await async_client.post(
|
||||
"/api/v1/auth/setup",
|
||||
json={
|
||||
"auth_enabled": True,
|
||||
"admin_username": "sessiontest3",
|
||||
"admin_password": "SessionPass3!",
|
||||
},
|
||||
)
|
||||
await _set_session_max_hours(db_session, "5000") # would be ~208 days
|
||||
|
||||
before = int(time.time())
|
||||
response = await async_client.post(
|
||||
"/api/v1/auth/login",
|
||||
json={"username": "sessiontest3", "password": "SessionPass3!"},
|
||||
)
|
||||
after = int(time.time())
|
||||
|
||||
assert response.status_code == 200
|
||||
token = response.json()["access_token"]
|
||||
decoded = jwt.decode(token, SECRET_KEY, algorithms=[ALGORITHM])
|
||||
# Clamped to 30 days, not 5000 hours.
|
||||
expected_min = before + SESSION_MAX_HOURS_HARD_CEILING * 3600 - 60
|
||||
expected_max = after + SESSION_MAX_HOURS_HARD_CEILING * 3600 + 60
|
||||
assert expected_min <= decoded["exp"] <= expected_max
|
||||
|
||||
|
||||
class TestSettingsAPIExposesSessionMaxHours:
|
||||
"""The /settings API must round-trip session_max_hours as an int."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_default_is_24(self, async_client: AsyncClient, db_session: AsyncSession):
|
||||
await _set_session_max_hours(db_session, None)
|
||||
response = await async_client.get("/api/v1/settings/")
|
||||
assert response.status_code == 200
|
||||
assert response.json()["session_max_hours"] == 24
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_update_accepts_valid_value(self, async_client: AsyncClient, db_session: AsyncSession):
|
||||
response = await async_client.patch(
|
||||
"/api/v1/settings/",
|
||||
json={"session_max_hours": 168},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
assert response.json()["session_max_hours"] == 168
|
||||
# Persisted as the int's string form so the resolver round-trips.
|
||||
result = await db_session.execute(select(Settings).where(Settings.key == "session_max_hours"))
|
||||
row = result.scalar_one()
|
||||
assert row.value == "168"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_update_rejects_zero(self, async_client: AsyncClient):
|
||||
response = await async_client.patch(
|
||||
"/api/v1/settings/",
|
||||
json={"session_max_hours": 0},
|
||||
)
|
||||
assert response.status_code == 422
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_update_rejects_above_ceiling(self, async_client: AsyncClient):
|
||||
response = await async_client.patch(
|
||||
"/api/v1/settings/",
|
||||
json={"session_max_hours": SESSION_MAX_HOURS_HARD_CEILING + 1},
|
||||
)
|
||||
assert response.status_code == 422
|
||||
Reference in New Issue
Block a user