feat(api): add GET/PATCH /accounts/me/security endpoint
Fifth commit in the session-expiration-policy series. Surfaces the session-policy override controls to account owners. - schemas/account_security.py: NEW. SessionPolicyResponse returns both the override (Optional[int]) and the effective value (always present) plus the system min/max bounds, so the frontend can render the Custom-preset form without re-implementing the defaults logic. SessionPolicyUpdateRequest accepts NULL to clear an override. - endpoints/account_security.py: NEW. GET and PATCH on /me/security. Owner-only via require_account_owner. PATCH validates per-field bounds, then validates the effective idle <= absolute invariant (catching the partial-override case the DB CHECK can't see), then writes the row + an account.session_policy_update audit event with old/new/effective_old/effective_new payload. - router.py: registers the new router under _tenant_deps next to accounts.router. Tests added in test_session_policy.py (8 cases): - GET returns NULL overrides + Strict defaults + system bounds. - PATCH persists override; next login JWT reflects new values (60min/240min -> idle_max=3600, abs_max=14400 seconds). - PATCH rejects idle < min (422). - PATCH rejects absolute > max (422). - PATCH rejects idle > absolute when both are set (422). - PATCH rejects partial override that produces effective idle > effective absolute (idle=43200, absolute=NULL with default 20160). - Engineer-role user gets 403. - PATCH writes exactly one audit row with the expected payload shape. 16/16 in test_session_policy. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
138
backend/app/api/endpoints/account_security.py
Normal file
138
backend/app/api/endpoints/account_security.py
Normal file
@@ -0,0 +1,138 @@
|
|||||||
|
"""Account session-policy endpoints — owner-only.
|
||||||
|
|
||||||
|
GET /accounts/me/security — read the policy + system bounds.
|
||||||
|
PATCH /accounts/me/security — set or clear the per-account override.
|
||||||
|
|
||||||
|
POST /accounts/me/security/revoke-sessions lands in the next commit.
|
||||||
|
|
||||||
|
See docs/plans/2026-05-13-session-expiration-policy.md §4.7 / §4.11.
|
||||||
|
"""
|
||||||
|
from typing import Annotated
|
||||||
|
|
||||||
|
from fastapi import APIRouter, Depends, HTTPException, status
|
||||||
|
from sqlalchemy import select
|
||||||
|
from sqlalchemy.ext.asyncio import AsyncSession
|
||||||
|
|
||||||
|
from app.api.deps import require_account_owner
|
||||||
|
from app.core.admin_database import get_admin_db
|
||||||
|
from app.core.audit import log_audit
|
||||||
|
from app.core.config import settings
|
||||||
|
from app.core.security import resolve_session_policy
|
||||||
|
from app.models.account import Account
|
||||||
|
from app.models.user import User
|
||||||
|
from app.schemas.account_security import (
|
||||||
|
SessionPolicyResponse,
|
||||||
|
SessionPolicyUpdateRequest,
|
||||||
|
)
|
||||||
|
|
||||||
|
router = APIRouter(prefix="/accounts/me/security", tags=["account-security"])
|
||||||
|
|
||||||
|
|
||||||
|
def _policy_response(account: Account) -> SessionPolicyResponse:
|
||||||
|
eff_idle, eff_abs = resolve_session_policy(account)
|
||||||
|
return SessionPolicyResponse(
|
||||||
|
idle_minutes=account.session_idle_minutes,
|
||||||
|
absolute_minutes=account.session_absolute_minutes,
|
||||||
|
effective_idle_minutes=eff_idle,
|
||||||
|
effective_absolute_minutes=eff_abs,
|
||||||
|
idle_minutes_min=settings.SESSION_IDLE_MINUTES_MIN,
|
||||||
|
idle_minutes_max=settings.SESSION_IDLE_MINUTES_MAX,
|
||||||
|
absolute_minutes_min=settings.SESSION_ABSOLUTE_MINUTES_MIN,
|
||||||
|
absolute_minutes_max=settings.SESSION_ABSOLUTE_MINUTES_MAX,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
async def _load_account(db: AsyncSession, account_id) -> Account:
|
||||||
|
return (
|
||||||
|
await db.execute(select(Account).where(Account.id == account_id))
|
||||||
|
).scalar_one()
|
||||||
|
|
||||||
|
|
||||||
|
@router.get("", response_model=SessionPolicyResponse)
|
||||||
|
async def get_session_policy(
|
||||||
|
current_user: Annotated[User, Depends(require_account_owner)],
|
||||||
|
db: Annotated[AsyncSession, Depends(get_admin_db)],
|
||||||
|
):
|
||||||
|
account = await _load_account(db, current_user.account_id)
|
||||||
|
return _policy_response(account)
|
||||||
|
|
||||||
|
|
||||||
|
@router.patch("", response_model=SessionPolicyResponse)
|
||||||
|
async def update_session_policy(
|
||||||
|
body: SessionPolicyUpdateRequest,
|
||||||
|
current_user: Annotated[User, Depends(require_account_owner)],
|
||||||
|
db: Annotated[AsyncSession, Depends(get_admin_db)],
|
||||||
|
):
|
||||||
|
account = await _load_account(db, current_user.account_id)
|
||||||
|
|
||||||
|
# Snapshot effective values BEFORE change, for audit.
|
||||||
|
old_idle = account.session_idle_minutes
|
||||||
|
old_abs = account.session_absolute_minutes
|
||||||
|
effective_old_idle, effective_old_abs = resolve_session_policy(account)
|
||||||
|
|
||||||
|
new_idle = body.idle_minutes
|
||||||
|
new_abs = body.absolute_minutes
|
||||||
|
|
||||||
|
# Per-field bound checks. NULL clears the override and is always valid.
|
||||||
|
if new_idle is not None and not (
|
||||||
|
settings.SESSION_IDLE_MINUTES_MIN <= new_idle <= settings.SESSION_IDLE_MINUTES_MAX
|
||||||
|
):
|
||||||
|
raise HTTPException(
|
||||||
|
status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
|
||||||
|
detail=(
|
||||||
|
f"idle_minutes must be between {settings.SESSION_IDLE_MINUTES_MIN} "
|
||||||
|
f"and {settings.SESSION_IDLE_MINUTES_MAX}"
|
||||||
|
),
|
||||||
|
)
|
||||||
|
if new_abs is not None and not (
|
||||||
|
settings.SESSION_ABSOLUTE_MINUTES_MIN <= new_abs <= settings.SESSION_ABSOLUTE_MINUTES_MAX
|
||||||
|
):
|
||||||
|
raise HTTPException(
|
||||||
|
status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
|
||||||
|
detail=(
|
||||||
|
f"absolute_minutes must be between {settings.SESSION_ABSOLUTE_MINUTES_MIN} "
|
||||||
|
f"and {settings.SESSION_ABSOLUTE_MINUTES_MAX}"
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
# Effective-value invariant: idle must not exceed absolute after defaults.
|
||||||
|
# The DB CHECK only catches the both-set case; this catches the partial-
|
||||||
|
# override case where (e.g.) idle=43200 with absolute=NULL would yield an
|
||||||
|
# effective idle larger than the system default absolute.
|
||||||
|
effective_new_idle = new_idle if new_idle is not None else settings.SESSION_IDLE_MINUTES_DEFAULT
|
||||||
|
effective_new_abs = new_abs if new_abs is not None else settings.SESSION_ABSOLUTE_MINUTES_DEFAULT
|
||||||
|
if effective_new_idle > effective_new_abs:
|
||||||
|
raise HTTPException(
|
||||||
|
status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
|
||||||
|
detail=(
|
||||||
|
f"Effective idle ({effective_new_idle}min) cannot exceed effective "
|
||||||
|
f"absolute ({effective_new_abs}min)"
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
account.session_idle_minutes = new_idle
|
||||||
|
account.session_absolute_minutes = new_abs
|
||||||
|
|
||||||
|
await log_audit(
|
||||||
|
db,
|
||||||
|
user_id=current_user.id,
|
||||||
|
account_id=account.id,
|
||||||
|
action="account.session_policy_update",
|
||||||
|
resource_type="account",
|
||||||
|
resource_id=account.id,
|
||||||
|
details={
|
||||||
|
"old": {"idle_minutes": old_idle, "absolute_minutes": old_abs},
|
||||||
|
"new": {"idle_minutes": new_idle, "absolute_minutes": new_abs},
|
||||||
|
"effective_old": {
|
||||||
|
"idle_minutes": effective_old_idle,
|
||||||
|
"absolute_minutes": effective_old_abs,
|
||||||
|
},
|
||||||
|
"effective_new": {
|
||||||
|
"idle_minutes": effective_new_idle,
|
||||||
|
"absolute_minutes": effective_new_abs,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
)
|
||||||
|
await db.commit()
|
||||||
|
await db.refresh(account)
|
||||||
|
return _policy_response(account)
|
||||||
@@ -72,6 +72,7 @@ from app.api.endpoints import (
|
|||||||
webhooks,
|
webhooks,
|
||||||
accounts,
|
accounts,
|
||||||
account_invite_lookup,
|
account_invite_lookup,
|
||||||
|
account_security,
|
||||||
)
|
)
|
||||||
|
|
||||||
api_router = APIRouter()
|
api_router = APIRouter()
|
||||||
@@ -144,6 +145,7 @@ api_router.include_router(folders.router, dependencies=_tenant_deps)
|
|||||||
api_router.include_router(step_categories.router, dependencies=_pro_deps)
|
api_router.include_router(step_categories.router, dependencies=_pro_deps)
|
||||||
api_router.include_router(steps.router, dependencies=_pro_deps)
|
api_router.include_router(steps.router, dependencies=_pro_deps)
|
||||||
api_router.include_router(accounts.router, dependencies=_tenant_deps)
|
api_router.include_router(accounts.router, dependencies=_tenant_deps)
|
||||||
|
api_router.include_router(account_security.router, dependencies=_tenant_deps)
|
||||||
api_router.include_router(shares.router, dependencies=_tenant_deps)
|
api_router.include_router(shares.router, dependencies=_tenant_deps)
|
||||||
api_router.include_router(tree_markdown.router, dependencies=_tenant_deps)
|
api_router.include_router(tree_markdown.router, dependencies=_tenant_deps)
|
||||||
api_router.include_router(ratings.router, dependencies=_tenant_deps)
|
api_router.include_router(ratings.router, dependencies=_tenant_deps)
|
||||||
|
|||||||
56
backend/app/schemas/account_security.py
Normal file
56
backend/app/schemas/account_security.py
Normal file
@@ -0,0 +1,56 @@
|
|||||||
|
"""Schemas for /accounts/me/security — session-policy management.
|
||||||
|
|
||||||
|
See docs/plans/2026-05-13-session-expiration-policy.md §4.7 and §4.11.
|
||||||
|
"""
|
||||||
|
from typing import Literal, Optional
|
||||||
|
|
||||||
|
from pydantic import BaseModel, Field
|
||||||
|
|
||||||
|
|
||||||
|
class SessionPolicyResponse(BaseModel):
|
||||||
|
"""GET /accounts/me/security — the policy in effect for this account.
|
||||||
|
|
||||||
|
Surfaces both the override (which may be NULL) and the effective value
|
||||||
|
(after defaults applied) so the frontend can show the current state
|
||||||
|
without re-implementing the defaults logic.
|
||||||
|
"""
|
||||||
|
|
||||||
|
# Per-account override values, NULL = "use system default."
|
||||||
|
idle_minutes: Optional[int] = Field(
|
||||||
|
default=None,
|
||||||
|
description="Account override; NULL means use the system default.",
|
||||||
|
)
|
||||||
|
absolute_minutes: Optional[int] = Field(default=None)
|
||||||
|
|
||||||
|
# Effective values after defaults applied (always non-NULL).
|
||||||
|
effective_idle_minutes: int
|
||||||
|
effective_absolute_minutes: int
|
||||||
|
|
||||||
|
# System-imposed bounds for the Custom-preset form inputs.
|
||||||
|
idle_minutes_min: int
|
||||||
|
idle_minutes_max: int
|
||||||
|
absolute_minutes_min: int
|
||||||
|
absolute_minutes_max: int
|
||||||
|
|
||||||
|
|
||||||
|
class SessionPolicyUpdateRequest(BaseModel):
|
||||||
|
"""PATCH /accounts/me/security — set or clear the per-account override.
|
||||||
|
|
||||||
|
Pass `null` for either field to clear the override and fall back to the
|
||||||
|
system default. Both bounds checks and the idle <= absolute invariant
|
||||||
|
are validated against the *effective* values at the endpoint, since the
|
||||||
|
DB CHECK constraint only covers the both-set case.
|
||||||
|
"""
|
||||||
|
|
||||||
|
idle_minutes: Optional[int] = None
|
||||||
|
absolute_minutes: Optional[int] = None
|
||||||
|
|
||||||
|
|
||||||
|
class RevokeSessionsRequest(BaseModel):
|
||||||
|
"""POST /accounts/me/security/revoke-sessions — bulk-revoke refresh tokens."""
|
||||||
|
|
||||||
|
scope: Literal["all", "others"] = "all"
|
||||||
|
|
||||||
|
|
||||||
|
class RevokeSessionsResponse(BaseModel):
|
||||||
|
revoked_count: int
|
||||||
@@ -7,6 +7,7 @@ This file grows across commits:
|
|||||||
- Commit 2: error-detail taxonomy (#11 + wrong-type + bad-signature)
|
- Commit 2: error-detail taxonomy (#11 + wrong-type + bad-signature)
|
||||||
- Commit 3: claims embedded at login + response fields surfaced (#1, #14)
|
- Commit 3: claims embedded at login + response fields surfaced (#1, #14)
|
||||||
- Commit 4: absolute-cap enforcement + grandfather path (#8, #9, #12)
|
- Commit 4: absolute-cap enforcement + grandfather path (#8, #9, #12)
|
||||||
|
- Commit 5: GET/PATCH /accounts/me/security (#2, #3, #4, #5, #7, #16)
|
||||||
"""
|
"""
|
||||||
|
|
||||||
import uuid
|
import uuid
|
||||||
@@ -196,6 +197,187 @@ def _backdate_auth_time(refresh_token: str, *, seconds_back: int) -> str:
|
|||||||
return jwt.encode(payload, settings.SECRET_KEY, algorithm=settings.ALGORITHM)
|
return jwt.encode(payload, settings.SECRET_KEY, algorithm=settings.ALGORITHM)
|
||||||
|
|
||||||
|
|
||||||
|
class TestSessionPolicyEndpoint:
|
||||||
|
"""§6 tests #2, #3, #4, #5, #7, #16 — GET/PATCH /accounts/me/security."""
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_get_returns_defaults_and_bounds(
|
||||||
|
self, client: AsyncClient, auth_headers: dict
|
||||||
|
):
|
||||||
|
response = await client.get(
|
||||||
|
"/api/v1/accounts/me/security", headers=auth_headers
|
||||||
|
)
|
||||||
|
assert response.status_code == 200, response.json()
|
||||||
|
body = response.json()
|
||||||
|
|
||||||
|
# No override yet -> effective values are the system defaults.
|
||||||
|
assert body["idle_minutes"] is None
|
||||||
|
assert body["absolute_minutes"] is None
|
||||||
|
assert body["effective_idle_minutes"] == 4320 # 3d Strict default
|
||||||
|
assert body["effective_absolute_minutes"] == 20160 # 14d
|
||||||
|
assert body["idle_minutes_min"] == 15
|
||||||
|
assert body["idle_minutes_max"] == 43200
|
||||||
|
assert body["absolute_minutes_min"] == 60
|
||||||
|
assert body["absolute_minutes_max"] == 129600
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_patch_persists_override_and_returns_new_state(
|
||||||
|
self, client: AsyncClient, auth_headers: dict
|
||||||
|
):
|
||||||
|
response = await client.patch(
|
||||||
|
"/api/v1/accounts/me/security",
|
||||||
|
headers=auth_headers,
|
||||||
|
json={"idle_minutes": 60, "absolute_minutes": 240},
|
||||||
|
)
|
||||||
|
assert response.status_code == 200, response.json()
|
||||||
|
body = response.json()
|
||||||
|
assert body["idle_minutes"] == 60
|
||||||
|
assert body["absolute_minutes"] == 240
|
||||||
|
assert body["effective_idle_minutes"] == 60
|
||||||
|
assert body["effective_absolute_minutes"] == 240
|
||||||
|
|
||||||
|
# Next login picks up the new policy.
|
||||||
|
login_resp = await client.post(
|
||||||
|
"/api/v1/auth/login/json",
|
||||||
|
json={"email": "test@example.com", "password": "TestPassword123!"},
|
||||||
|
)
|
||||||
|
new_payload = jwt.decode(
|
||||||
|
login_resp.json()["refresh_token"],
|
||||||
|
settings.SECRET_KEY,
|
||||||
|
algorithms=[settings.ALGORITHM],
|
||||||
|
)
|
||||||
|
assert new_payload["idle_max"] == 60 * 60 # 3600 seconds
|
||||||
|
assert new_payload["abs_max"] == 240 * 60 # 14400 seconds
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_patch_rejects_idle_below_min(
|
||||||
|
self, client: AsyncClient, auth_headers: dict
|
||||||
|
):
|
||||||
|
response = await client.patch(
|
||||||
|
"/api/v1/accounts/me/security",
|
||||||
|
headers=auth_headers,
|
||||||
|
json={"idle_minutes": 5, "absolute_minutes": 60},
|
||||||
|
)
|
||||||
|
assert response.status_code == 422
|
||||||
|
assert "idle_minutes" in response.json()["detail"]
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_patch_rejects_absolute_above_max(
|
||||||
|
self, client: AsyncClient, auth_headers: dict
|
||||||
|
):
|
||||||
|
response = await client.patch(
|
||||||
|
"/api/v1/accounts/me/security",
|
||||||
|
headers=auth_headers,
|
||||||
|
json={"absolute_minutes": 200000},
|
||||||
|
)
|
||||||
|
assert response.status_code == 422
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_patch_rejects_idle_greater_than_absolute_both_set(
|
||||||
|
self, client: AsyncClient, auth_headers: dict
|
||||||
|
):
|
||||||
|
response = await client.patch(
|
||||||
|
"/api/v1/accounts/me/security",
|
||||||
|
headers=auth_headers,
|
||||||
|
json={"idle_minutes": 300, "absolute_minutes": 120},
|
||||||
|
)
|
||||||
|
assert response.status_code == 422
|
||||||
|
assert "exceed" in response.json()["detail"].lower()
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_patch_rejects_partial_override_when_effective_invalid(
|
||||||
|
self, client: AsyncClient, auth_headers: dict
|
||||||
|
):
|
||||||
|
"""§6 test #5 — partial override: idle=43200, absolute=NULL ->
|
||||||
|
effective idle (43200) > effective absolute (20160 default) -> 422.
|
||||||
|
"""
|
||||||
|
response = await client.patch(
|
||||||
|
"/api/v1/accounts/me/security",
|
||||||
|
headers=auth_headers,
|
||||||
|
json={"idle_minutes": 43200, "absolute_minutes": None},
|
||||||
|
)
|
||||||
|
assert response.status_code == 422
|
||||||
|
assert "exceed" in response.json()["detail"].lower()
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_non_owner_cannot_patch(
|
||||||
|
self, client: AsyncClient, test_user: dict, test_db
|
||||||
|
):
|
||||||
|
"""§6 test #7 — engineer role is forbidden."""
|
||||||
|
from app.models.user import User
|
||||||
|
from sqlalchemy import select
|
||||||
|
|
||||||
|
# Add a second user in the same account with account_role=engineer.
|
||||||
|
result = await test_db.execute(
|
||||||
|
select(User).where(User.email == test_user["email"])
|
||||||
|
)
|
||||||
|
owner = result.scalar_one()
|
||||||
|
engineer = User(
|
||||||
|
email="engineer-policy@example.com",
|
||||||
|
password_hash=owner.password_hash, # reuse the bcrypt hash
|
||||||
|
name="Engineer",
|
||||||
|
role="engineer",
|
||||||
|
is_super_admin=False,
|
||||||
|
is_active=True,
|
||||||
|
account_id=owner.account_id,
|
||||||
|
account_role="engineer",
|
||||||
|
email_verified_at=datetime.now(timezone.utc),
|
||||||
|
)
|
||||||
|
test_db.add(engineer)
|
||||||
|
await test_db.commit()
|
||||||
|
|
||||||
|
login_resp = await client.post(
|
||||||
|
"/api/v1/auth/login/json",
|
||||||
|
json={
|
||||||
|
"email": "engineer-policy@example.com",
|
||||||
|
"password": test_user["password"],
|
||||||
|
},
|
||||||
|
)
|
||||||
|
assert login_resp.status_code == 200
|
||||||
|
engineer_headers = {
|
||||||
|
"Authorization": f"Bearer {login_resp.json()['access_token']}"
|
||||||
|
}
|
||||||
|
|
||||||
|
response = await client.patch(
|
||||||
|
"/api/v1/accounts/me/security",
|
||||||
|
headers=engineer_headers,
|
||||||
|
json={"idle_minutes": 60, "absolute_minutes": 240},
|
||||||
|
)
|
||||||
|
assert response.status_code == 403
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_patch_writes_audit_row(
|
||||||
|
self, client: AsyncClient, auth_headers: dict, test_db
|
||||||
|
):
|
||||||
|
"""§6 test #16 — PATCH emits one account.session_policy_update
|
||||||
|
audit event with old/new + effective_old/new payload.
|
||||||
|
"""
|
||||||
|
from app.models.audit_log import AuditLog
|
||||||
|
from sqlalchemy import select
|
||||||
|
|
||||||
|
response = await client.patch(
|
||||||
|
"/api/v1/accounts/me/security",
|
||||||
|
headers=auth_headers,
|
||||||
|
json={"idle_minutes": 120, "absolute_minutes": 480},
|
||||||
|
)
|
||||||
|
assert response.status_code == 200
|
||||||
|
|
||||||
|
result = await test_db.execute(
|
||||||
|
select(AuditLog).where(AuditLog.action == "account.session_policy_update")
|
||||||
|
)
|
||||||
|
rows = result.scalars().all()
|
||||||
|
assert len(rows) == 1
|
||||||
|
entry = rows[0]
|
||||||
|
assert entry.resource_type == "account"
|
||||||
|
assert entry.details["new"] == {"idle_minutes": 120, "absolute_minutes": 480}
|
||||||
|
assert entry.details["effective_new"] == {
|
||||||
|
"idle_minutes": 120,
|
||||||
|
"absolute_minutes": 480,
|
||||||
|
}
|
||||||
|
assert entry.details["effective_old"]["idle_minutes"] == 4320 # default
|
||||||
|
assert entry.details["effective_old"]["absolute_minutes"] == 20160
|
||||||
|
|
||||||
|
|
||||||
class TestAbsoluteCap:
|
class TestAbsoluteCap:
|
||||||
"""§6 tests #8, #9, #12 — absolute-cap enforcement and grandfather path."""
|
"""§6 tests #8, #9, #12 — absolute-cap enforcement and grandfather path."""
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user