fix: high-severity security hardening (Phase B permissions audit)
Phase B addresses 7 high-severity gaps from the permissions audit: - B1: Enforce tree access check on session start via can_access_tree - B2: Replace all inline permission helpers with centralized permissions.py - B3: Fix require_engineer_or_admin to check is_team_admin before role - B4: Add is_active field on User with enforcement in get_current_active_user - B5: Add admin user management endpoints (list, get, role, team-admin, deactivate, activate) - B6: Add rate limiting on auth/invite endpoints via slowapi (disabled in DEBUG) - B7: Implement refresh token rotation with JTI-based revocation and meaningful logout Also reduces access token TTL from 15 to 5 minutes and updates CLAUDE.md with SaaS/MSP context for future planning sessions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -1,29 +1,46 @@
|
||||
from datetime import datetime, timezone
|
||||
from typing import Annotated
|
||||
from fastapi import APIRouter, Depends, HTTPException, status
|
||||
from fastapi import APIRouter, Depends, HTTPException, status, Request
|
||||
from fastapi.security import OAuth2PasswordRequestForm
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
from sqlalchemy import select
|
||||
|
||||
from app.core.config import settings
|
||||
from app.core.database import get_db
|
||||
from app.core.rate_limit import limiter
|
||||
from app.core.security import (
|
||||
verify_password,
|
||||
get_password_hash,
|
||||
create_access_token,
|
||||
create_refresh_token,
|
||||
decode_token,
|
||||
hash_token,
|
||||
)
|
||||
from app.models.user import User
|
||||
from app.models.invite_code import InviteCode
|
||||
from app.models.refresh_token import RefreshToken
|
||||
from app.schemas.user import UserCreate, UserResponse, UserLogin
|
||||
from app.schemas.token import Token
|
||||
from app.api.deps import get_current_user, get_refresh_token_payload
|
||||
from app.api.deps import get_current_active_user, get_refresh_token_payload
|
||||
|
||||
router = APIRouter(prefix="/auth", tags=["authentication"])
|
||||
|
||||
|
||||
async def _store_refresh_token(db: AsyncSession, refresh_token_str: str, user_id) -> None:
|
||||
"""Decode a refresh token JWT and store its hash in the database."""
|
||||
payload = decode_token(refresh_token_str)
|
||||
if payload and payload.get("jti"):
|
||||
token_record = RefreshToken(
|
||||
token_hash=hash_token(payload["jti"]),
|
||||
user_id=user_id,
|
||||
expires_at=datetime.fromtimestamp(payload["exp"], tz=timezone.utc),
|
||||
)
|
||||
db.add(token_record)
|
||||
|
||||
|
||||
@router.post("/register", response_model=UserResponse, status_code=status.HTTP_201_CREATED)
|
||||
@limiter.limit("3/minute")
|
||||
async def register(
|
||||
request: Request,
|
||||
user_data: UserCreate,
|
||||
db: Annotated[AsyncSession, Depends(get_db)]
|
||||
):
|
||||
@@ -92,7 +109,9 @@ async def register(
|
||||
|
||||
|
||||
@router.post("/login", response_model=Token)
|
||||
@limiter.limit("5/minute")
|
||||
async def login(
|
||||
request: Request,
|
||||
form_data: Annotated[OAuth2PasswordRequestForm, Depends()],
|
||||
db: Annotated[AsyncSession, Depends(get_db)]
|
||||
):
|
||||
@@ -110,21 +129,26 @@ async def login(
|
||||
|
||||
# Update last login
|
||||
user.last_login = datetime.now(timezone.utc)
|
||||
await db.commit()
|
||||
|
||||
# Create tokens
|
||||
access_token = create_access_token(data={"sub": str(user.id)})
|
||||
refresh_token = create_refresh_token(data={"sub": str(user.id)})
|
||||
refresh_token_str = create_refresh_token(data={"sub": str(user.id)})
|
||||
|
||||
# Store refresh token hash in DB
|
||||
await _store_refresh_token(db, refresh_token_str, user.id)
|
||||
await db.commit()
|
||||
|
||||
return Token(
|
||||
access_token=access_token,
|
||||
refresh_token=refresh_token,
|
||||
refresh_token=refresh_token_str,
|
||||
token_type="bearer"
|
||||
)
|
||||
|
||||
|
||||
@router.post("/login/json", response_model=Token)
|
||||
@limiter.limit("5/minute")
|
||||
async def login_json(
|
||||
request: Request,
|
||||
credentials: UserLogin,
|
||||
db: Annotated[AsyncSession, Depends(get_db)]
|
||||
):
|
||||
@@ -139,25 +163,50 @@ async def login_json(
|
||||
)
|
||||
|
||||
user.last_login = datetime.now(timezone.utc)
|
||||
await db.commit()
|
||||
|
||||
access_token = create_access_token(data={"sub": str(user.id)})
|
||||
refresh_token = create_refresh_token(data={"sub": str(user.id)})
|
||||
refresh_token_str = create_refresh_token(data={"sub": str(user.id)})
|
||||
|
||||
# Store refresh token hash in DB
|
||||
await _store_refresh_token(db, refresh_token_str, user.id)
|
||||
await db.commit()
|
||||
|
||||
return Token(
|
||||
access_token=access_token,
|
||||
refresh_token=refresh_token,
|
||||
refresh_token=refresh_token_str,
|
||||
token_type="bearer"
|
||||
)
|
||||
|
||||
|
||||
@router.post("/refresh", response_model=Token)
|
||||
@limiter.limit("10/minute")
|
||||
async def refresh_token(
|
||||
request: Request,
|
||||
payload: Annotated[dict, Depends(get_refresh_token_payload)],
|
||||
db: Annotated[AsyncSession, Depends(get_db)]
|
||||
):
|
||||
"""Refresh access token using refresh token."""
|
||||
"""Refresh access token using refresh token (rotation: old token is revoked)."""
|
||||
user_id = payload.get("sub")
|
||||
jti = payload.get("jti")
|
||||
|
||||
# Validate refresh token hasn't been revoked
|
||||
if jti:
|
||||
token_hash = hash_token(jti)
|
||||
result = await db.execute(
|
||||
select(RefreshToken).where(RefreshToken.token_hash == token_hash)
|
||||
)
|
||||
stored_token = result.scalar_one_or_none()
|
||||
|
||||
if stored_token and stored_token.is_revoked:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_401_UNAUTHORIZED,
|
||||
detail="Refresh token has been revoked"
|
||||
)
|
||||
|
||||
# Revoke the old refresh token (token rotation)
|
||||
if stored_token:
|
||||
stored_token.revoked_at = datetime.now(timezone.utc)
|
||||
|
||||
result = await db.execute(select(User).where(User.id == user_id))
|
||||
user = result.scalar_one_or_none()
|
||||
|
||||
@@ -168,26 +217,42 @@ async def refresh_token(
|
||||
)
|
||||
|
||||
access_token = create_access_token(data={"sub": str(user.id)})
|
||||
new_refresh_token = create_refresh_token(data={"sub": str(user.id)})
|
||||
new_refresh_token_str = create_refresh_token(data={"sub": str(user.id)})
|
||||
|
||||
# Store new refresh token
|
||||
await _store_refresh_token(db, new_refresh_token_str, user.id)
|
||||
await db.commit()
|
||||
|
||||
return Token(
|
||||
access_token=access_token,
|
||||
refresh_token=new_refresh_token,
|
||||
refresh_token=new_refresh_token_str,
|
||||
token_type="bearer"
|
||||
)
|
||||
|
||||
|
||||
@router.get("/me", response_model=UserResponse)
|
||||
async def get_me(
|
||||
current_user: Annotated[User, Depends(get_current_user)]
|
||||
current_user: Annotated[User, Depends(get_current_active_user)]
|
||||
):
|
||||
"""Get current authenticated user."""
|
||||
return current_user
|
||||
|
||||
|
||||
@router.post("/logout")
|
||||
async def logout():
|
||||
"""Logout user (client should discard tokens)."""
|
||||
# JWT tokens are stateless, so logout is handled client-side
|
||||
# In a production app, you might want to blacklist the token
|
||||
async def logout(
|
||||
payload: Annotated[dict, Depends(get_refresh_token_payload)],
|
||||
db: Annotated[AsyncSession, Depends(get_db)]
|
||||
):
|
||||
"""Logout user by revoking the refresh token."""
|
||||
jti = payload.get("jti")
|
||||
if jti:
|
||||
token_hash = hash_token(jti)
|
||||
result = await db.execute(
|
||||
select(RefreshToken).where(RefreshToken.token_hash == token_hash)
|
||||
)
|
||||
stored_token = result.scalar_one_or_none()
|
||||
if stored_token and not stored_token.is_revoked:
|
||||
stored_token.revoked_at = datetime.now(timezone.utc)
|
||||
await db.commit()
|
||||
|
||||
return {"message": "Successfully logged out"}
|
||||
|
||||
Reference in New Issue
Block a user