fix(security): remediate telecom compliance findings
This commit is contained in:
@@ -5,11 +5,13 @@
|
||||
|
||||
# ── 应用 ──
|
||||
APP_NAME=企迹-政企周报管理系统
|
||||
DEBUG=true
|
||||
SECRET_KEY=change-me-to-a-random-string-in-production
|
||||
ENVIRONMENT=development
|
||||
DEBUG=false
|
||||
# Use a random value of at least 32 characters. Never use a default in production.
|
||||
SECRET_KEY=replace-with-a-random-secret-of-at-least-32-characters
|
||||
|
||||
# ── PostgreSQL (已有基础设施,填写实际连接信息) ──
|
||||
DATABASE_URL=postgresql+asyncpg://postgres:postgres@localhost:5432/qiji
|
||||
DATABASE_URL=postgresql+asyncpg://qiji_app:replace-with-a-strong-password@localhost:5432/qiji
|
||||
|
||||
# ── JWT ──
|
||||
JWT_ALGORITHM=HS256
|
||||
@@ -25,10 +27,11 @@ CASDOOR_APPLICATION=qiji-weekly-report
|
||||
|
||||
# ── MinIO (已有,填写实际部署地址和密钥) ──
|
||||
MINIO_ENDPOINT=localhost:9000
|
||||
MINIO_ACCESS_KEY=minioadmin
|
||||
MINIO_SECRET_KEY=minioadmin
|
||||
MINIO_ACCESS_KEY=qiji-app
|
||||
MINIO_SECRET_KEY=replace-with-a-strong-minio-secret
|
||||
MINIO_BUCKET=qiji-photos
|
||||
MINIO_SECURE=false
|
||||
# Set false only when connecting to a local development MinIO instance over the internal Docker network.
|
||||
MINIO_SECURE=true
|
||||
|
||||
# ── 企业微信 (需在企微管理后台创建自建应用后获取) ──
|
||||
WECOM_CORP_ID=your-corp-id
|
||||
|
||||
+7
-27
@@ -58,36 +58,16 @@ async def wecom_login(req: WecomLoginRequest, db: AsyncSession = Depends(get_db)
|
||||
role=user.role,
|
||||
)
|
||||
|
||||
# Not bound yet — return a redirect URL to Casdoor for binding
|
||||
casdoor_auth_url = (
|
||||
f"{settings.CASDOOR_ENDPOINT}/login/oauth/authorize"
|
||||
f"?client_id={settings.CASDOOR_CLIENT_ID}"
|
||||
f"&response_type=code"
|
||||
f"&redirect_uri={settings.CORS_ORIGINS[0]}/bind-wecom"
|
||||
f"&scope=openid+profile"
|
||||
f"&state={wecom_userid}"
|
||||
raise HTTPException(
|
||||
status_code=403,
|
||||
detail="企业微信账号尚未绑定。请在企业微信内使用一次性绑定链接完成绑定。",
|
||||
)
|
||||
return {"need_bind": True, "casdoor_url": casdoor_auth_url, "wecom_userid": wecom_userid}
|
||||
|
||||
|
||||
@router.post("/bind-wecom")
|
||||
async def bind_wecom(req: WecomBindRequest, db: AsyncSession = Depends(get_db)):
|
||||
"""Bind Casdoor account with WeChat Work userid after OIDC redirect."""
|
||||
userinfo = await exchange_casdoor_code(req.casdoor_code)
|
||||
if not userinfo:
|
||||
raise HTTPException(status_code=400, detail="Failed to exchange casdoor code")
|
||||
|
||||
casdoor_id = userinfo.get("sub") or userinfo.get("id")
|
||||
wecom_userid = req.wecom_userid or userinfo.get("state", "")
|
||||
|
||||
user = await bind_wecom_user(db, casdoor_id, wecom_userid)
|
||||
if not user:
|
||||
raise HTTPException(status_code=404, detail="User not found")
|
||||
|
||||
token = build_token_for_user(user)
|
||||
return TokenResponse(
|
||||
access_token=token,
|
||||
user_id=str(user.id),
|
||||
name=user.name,
|
||||
role=user.role,
|
||||
"""Deprecated unsafe binding flow. Use /wecom/bind-confirm with a one-time token."""
|
||||
raise HTTPException(
|
||||
status_code=410,
|
||||
detail="该绑定接口已停用,请使用企业微信一次性绑定链接。",
|
||||
)
|
||||
|
||||
@@ -46,7 +46,10 @@ async def get_notification_time(db: AsyncSession) -> str:
|
||||
|
||||
|
||||
@router.get("")
|
||||
async def list_config(db: AsyncSession = Depends(get_db)):
|
||||
async def list_config(
|
||||
current_user: dict = Depends(require_director),
|
||||
db: AsyncSession = Depends(get_db),
|
||||
):
|
||||
"""Return all system config as {key: value} dict."""
|
||||
result = await db.execute(select(SystemConfig))
|
||||
rows = result.scalars().all()
|
||||
|
||||
@@ -1,10 +1,48 @@
|
||||
import uuid
|
||||
from fastapi import APIRouter, Depends
|
||||
from datetime import date
|
||||
from pathlib import PurePosixPath
|
||||
from fastapi import APIRouter, Depends, HTTPException
|
||||
from sqlalchemy import select
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
from app.database import get_db
|
||||
from app.middleware.auth import get_current_user, require_any_role
|
||||
from app.services.minio_client import generate_presigned_upload_url, generate_presigned_download_url
|
||||
from app.models.visit import Visit
|
||||
|
||||
router = APIRouter(prefix="/upload", tags=["Upload"])
|
||||
|
||||
_IMAGE_TYPES = {
|
||||
"image/jpeg": "jpg",
|
||||
"image/png": "png",
|
||||
"image/webp": "webp",
|
||||
}
|
||||
|
||||
|
||||
def validate_image_upload(filename: str, content_type: str) -> str:
|
||||
"""Validate client-declared image metadata before issuing a short-lived PUT URL."""
|
||||
extension = _IMAGE_TYPES.get(content_type.lower())
|
||||
if not extension:
|
||||
raise HTTPException(status_code=400, detail="Only JPEG, PNG, and WebP images are supported")
|
||||
if PurePosixPath(filename).suffix.lower() not in {".jpg", ".jpeg", ".png", ".webp"}:
|
||||
raise HTTPException(status_code=400, detail="Invalid image filename extension")
|
||||
return extension
|
||||
|
||||
|
||||
def build_object_key(user_id: str, extension: str) -> str:
|
||||
return f"uploads/{user_id}/{date.today().isoformat()}/{uuid.uuid4().hex}.{extension}"
|
||||
|
||||
|
||||
def is_owned_upload_key(object_key: str, user_id: str) -> bool:
|
||||
path = PurePosixPath(object_key)
|
||||
parts = path.parts
|
||||
return (
|
||||
len(parts) == 4
|
||||
and parts[0] == "uploads"
|
||||
and parts[1] == user_id
|
||||
and ".." not in parts
|
||||
and path.suffix.lower() in {".jpg", ".png", ".webp"}
|
||||
)
|
||||
|
||||
|
||||
@router.post("/presigned-url")
|
||||
async def get_presigned_upload_url(
|
||||
@@ -13,10 +51,8 @@ async def get_presigned_upload_url(
|
||||
current_user: dict = Depends(require_any_role),
|
||||
):
|
||||
"""Get a presigned PUT URL for direct MinIO upload."""
|
||||
import datetime
|
||||
today = datetime.date.today().isoformat()
|
||||
user_id = current_user["user_id"][:8]
|
||||
object_key = f"{today}/{user_id}/{uuid.uuid4()}.jpg"
|
||||
extension = validate_image_upload(filename, content_type)
|
||||
object_key = build_object_key(current_user["user_id"], extension)
|
||||
|
||||
url = generate_presigned_upload_url(object_key)
|
||||
|
||||
@@ -30,7 +66,14 @@ async def get_presigned_upload_url(
|
||||
async def get_presigned_download_url(
|
||||
object_key: str,
|
||||
current_user: dict = Depends(get_current_user),
|
||||
db: AsyncSession = Depends(get_db),
|
||||
):
|
||||
"""Get a presigned GET URL for viewing a photo (1 hour validity)."""
|
||||
result = await db.execute(select(Visit).where(Visit.photos.any(object_key)).limit(1))
|
||||
visit = result.scalar_one_or_none()
|
||||
if not visit:
|
||||
raise HTTPException(status_code=404, detail="Photo not found")
|
||||
if current_user["role"] == "manager" and str(visit.manager_id) != current_user["user_id"]:
|
||||
raise HTTPException(status_code=403, detail="Access denied")
|
||||
url = generate_presigned_download_url(object_key)
|
||||
return {"download_url": url}
|
||||
|
||||
@@ -12,11 +12,21 @@ from app.models.user import User
|
||||
from app.schemas.visit import VisitCreate, VisitUpdate, VisitOut, VisitListOut
|
||||
from app.utils.timezone import today_cst, parse_date
|
||||
from app.services.minio_client import delete_objects
|
||||
from app.api.upload import is_owned_upload_key
|
||||
from app.utils.edit_log import compute_diff, append_entry, init_entry
|
||||
|
||||
router = APIRouter(prefix="/visits", tags=["Visits"])
|
||||
|
||||
|
||||
def _validate_photo_keys(photo_keys: list[str], user_id: str, existing_keys: list[str] | None = None) -> None:
|
||||
if len(photo_keys) > 9:
|
||||
raise HTTPException(status_code=400, detail="A visit may contain at most 9 photos")
|
||||
existing = set(existing_keys or [])
|
||||
for key in photo_keys:
|
||||
if key not in existing and not is_owned_upload_key(key, user_id):
|
||||
raise HTTPException(status_code=400, detail="Invalid or unauthorized photo reference")
|
||||
|
||||
|
||||
async def _enrich_visit(visit: Visit, db: AsyncSession) -> dict:
|
||||
"""Enrich a visit record with customer/manager names."""
|
||||
customer_name = None
|
||||
@@ -137,6 +147,7 @@ async def create_visit(
|
||||
db: AsyncSession = Depends(get_db),
|
||||
):
|
||||
"""Create a visit record. If companions are selected, creates draft copies for them."""
|
||||
_validate_photo_keys(data.photos, current_user["user_id"])
|
||||
visit = Visit(
|
||||
customer_id=data.customer_id,
|
||||
visit_date=parse_date(data.visit_date),
|
||||
@@ -209,6 +220,9 @@ async def update_visit(
|
||||
if current_user["role"] == "manager" and str(visit.manager_id) != current_user["user_id"]:
|
||||
raise HTTPException(status_code=403, detail="Access denied")
|
||||
|
||||
if data.photos is not None:
|
||||
_validate_photo_keys(data.photos, current_user["user_id"], visit.photos or [])
|
||||
|
||||
# Snapshot old values for diff
|
||||
old_snapshot = {
|
||||
"customer_id": str(visit.customer_id), "visit_date": str(visit.visit_date),
|
||||
|
||||
@@ -111,15 +111,15 @@ async def wecom_callback_event(request: Request):
|
||||
|
||||
import logging
|
||||
_log = logging.getLogger("wecom_callback")
|
||||
_log.warning(f"wecom msg: type={msg_type} from={from_user} content={repr(content)} event={event} key={event_key}")
|
||||
_log.info("wecom callback received: type=%s event=%s key=%s", msg_type, event, event_key)
|
||||
|
||||
# Triggers for binding
|
||||
if (msg_type == "text" and content == "绑定") or (msg_type == "event" and event == "click" and event_key == "BIND_ACCOUNT"):
|
||||
_log.warning(f"wecom bind triggered for {from_user}")
|
||||
_log.info("wecom bind triggered")
|
||||
_handle_bind_request(from_user)
|
||||
return PlainTextResponse(content="success")
|
||||
|
||||
_log.warning(f"wecom msg ignored (no match)")
|
||||
_log.info("wecom message ignored (no matching action)")
|
||||
return PlainTextResponse(content="success")
|
||||
|
||||
|
||||
|
||||
+30
-7
@@ -5,11 +5,13 @@ from typing import Optional
|
||||
class Settings(BaseSettings):
|
||||
# App
|
||||
APP_NAME: str = "企迹-政企周报管理系统"
|
||||
DEBUG: bool = True
|
||||
SECRET_KEY: str = "change-me-in-production"
|
||||
ENVIRONMENT: str = "production"
|
||||
DEBUG: bool = False
|
||||
SECRET_KEY: str = ""
|
||||
|
||||
# Database
|
||||
DATABASE_URL: str = "postgresql+asyncpg://postgres:postgres@localhost:5432/qiji"
|
||||
# A syntactically valid non-working placeholder keeps tooling imports side-effect free.
|
||||
DATABASE_URL: str = "postgresql+asyncpg://qiji_app:invalid@localhost:5432/qiji"
|
||||
|
||||
# JWT
|
||||
JWT_ALGORITHM: str = "HS256"
|
||||
@@ -24,11 +26,11 @@ class Settings(BaseSettings):
|
||||
CASDOOR_APPLICATION: str = "qiji-weekly-report"
|
||||
|
||||
# MinIO
|
||||
MINIO_ENDPOINT: str = "localhost:9000"
|
||||
MINIO_ACCESS_KEY: str = "minioadmin"
|
||||
MINIO_SECRET_KEY: str = "minioadmin"
|
||||
MINIO_ENDPOINT: str = ""
|
||||
MINIO_ACCESS_KEY: str = ""
|
||||
MINIO_SECRET_KEY: str = ""
|
||||
MINIO_BUCKET: str = "qiji-photos"
|
||||
MINIO_SECURE: bool = False
|
||||
MINIO_SECURE: bool = True
|
||||
|
||||
# WeChat Work
|
||||
WECOM_CORP_ID: str = ""
|
||||
@@ -53,3 +55,24 @@ class Settings(BaseSettings):
|
||||
|
||||
|
||||
settings = Settings()
|
||||
|
||||
|
||||
def validate_security_settings() -> None:
|
||||
"""Reject unsafe or incomplete configuration before the application starts."""
|
||||
if settings.ENVIRONMENT.lower() == "development":
|
||||
return
|
||||
|
||||
invalid = []
|
||||
if len(settings.SECRET_KEY) < 32 or settings.SECRET_KEY in {"change-me-in-production", ""}:
|
||||
invalid.append("SECRET_KEY")
|
||||
if not settings.DATABASE_URL or "postgres:postgres@" in settings.DATABASE_URL or ":invalid@" in settings.DATABASE_URL:
|
||||
invalid.append("DATABASE_URL")
|
||||
if not settings.MINIO_ENDPOINT or not settings.MINIO_ACCESS_KEY or not settings.MINIO_SECRET_KEY:
|
||||
invalid.append("MINIO configuration")
|
||||
if settings.MINIO_ACCESS_KEY == "minioadmin" or settings.MINIO_SECRET_KEY == "minioadmin":
|
||||
invalid.append("MINIO default credentials")
|
||||
if not settings.MINIO_SECURE:
|
||||
invalid.append("MINIO_SECURE")
|
||||
|
||||
if invalid:
|
||||
raise RuntimeError(f"Unsafe production configuration: {', '.join(invalid)}")
|
||||
|
||||
+2
-1
@@ -2,7 +2,7 @@ from contextlib import asynccontextmanager
|
||||
from fastapi import FastAPI
|
||||
from fastapi.middleware.cors import CORSMiddleware
|
||||
from sqlalchemy import select
|
||||
from app.config import settings
|
||||
from app.config import settings, validate_security_settings
|
||||
from app.database import engine, Base, async_session
|
||||
from app.api import router as api_router
|
||||
from app.api import auth, users, customers, visits, work_plans, mini_business, key_visits
|
||||
@@ -14,6 +14,7 @@ from app.services.scheduler_manager import start_scheduler, shutdown_scheduler
|
||||
|
||||
@asynccontextmanager
|
||||
async def lifespan(app: FastAPI):
|
||||
validate_security_settings()
|
||||
# Startup: create tables if not exists (for dev convenience)
|
||||
async with engine.begin() as conn:
|
||||
await conn.run_sync(Base.metadata.create_all)
|
||||
|
||||
@@ -49,12 +49,12 @@ async def exchange_casdoor_code(code: str) -> dict | None:
|
||||
"code": code,
|
||||
}, timeout=10)
|
||||
if resp.status_code != 200:
|
||||
logger.error(f"[Casdoor] token exchange failed: status={resp.status_code}, body={resp.text[:500]}")
|
||||
logger.error("[Casdoor] token exchange failed: status=%s", resp.status_code)
|
||||
return None
|
||||
token_data = resp.json()
|
||||
access_token = token_data.get("access_token", "")
|
||||
if not access_token:
|
||||
logger.error(f"[Casdoor] no access_token in response: {token_data}")
|
||||
logger.error("[Casdoor] token exchange response did not contain an access token")
|
||||
return None
|
||||
except Exception as e:
|
||||
logger.error(f"[Casdoor] token exchange exception: {e}")
|
||||
@@ -67,7 +67,7 @@ async def exchange_casdoor_code(code: str) -> dict | None:
|
||||
"Authorization": f"Bearer {access_token}"
|
||||
}, timeout=10)
|
||||
if resp.status_code != 200:
|
||||
logger.error(f"[Casdoor] userinfo failed: status={resp.status_code}, body={resp.text[:500]}")
|
||||
logger.error("[Casdoor] userinfo failed: status=%s", resp.status_code)
|
||||
return None
|
||||
return resp.json()
|
||||
except Exception as e:
|
||||
|
||||
@@ -0,0 +1,49 @@
|
||||
from fastapi import HTTPException
|
||||
|
||||
from app.api.upload import build_object_key, is_owned_upload_key, validate_image_upload
|
||||
from app.config import settings, validate_security_settings
|
||||
|
||||
|
||||
USER_ID = "a2d4447f-7b67-4304-ad9a-953007e75ef2"
|
||||
|
||||
|
||||
def test_build_object_key_is_scoped_to_the_authenticated_user():
|
||||
object_key = build_object_key(USER_ID, "jpg")
|
||||
|
||||
assert is_owned_upload_key(object_key, USER_ID)
|
||||
assert not is_owned_upload_key(object_key, "cf610b28-51a7-423d-a55a-730e193c9971")
|
||||
|
||||
|
||||
def test_upload_rejects_non_image_content_type():
|
||||
try:
|
||||
validate_image_upload("payload.exe", "application/octet-stream")
|
||||
except HTTPException as exc:
|
||||
assert exc.status_code == 400
|
||||
else:
|
||||
raise AssertionError("non-image uploads must be rejected")
|
||||
|
||||
|
||||
def test_upload_rejects_an_invalid_filename_extension():
|
||||
try:
|
||||
validate_image_upload("payload.txt", "image/jpeg")
|
||||
except HTTPException as exc:
|
||||
assert exc.status_code == 400
|
||||
else:
|
||||
raise AssertionError("invalid extensions must be rejected")
|
||||
|
||||
|
||||
def test_production_rejects_placeholder_database_configuration(monkeypatch):
|
||||
monkeypatch.setattr(settings, "ENVIRONMENT", "production")
|
||||
monkeypatch.setattr(settings, "SECRET_KEY", "a" * 32)
|
||||
monkeypatch.setattr(settings, "DATABASE_URL", "postgresql+asyncpg://qiji_app:invalid@localhost:5432/qiji")
|
||||
monkeypatch.setattr(settings, "MINIO_ENDPOINT", "minio.example.test:9000")
|
||||
monkeypatch.setattr(settings, "MINIO_ACCESS_KEY", "qiji-app")
|
||||
monkeypatch.setattr(settings, "MINIO_SECRET_KEY", "a-secure-minio-secret")
|
||||
monkeypatch.setattr(settings, "MINIO_SECURE", True)
|
||||
|
||||
try:
|
||||
validate_security_settings()
|
||||
except RuntimeError as exc:
|
||||
assert "DATABASE_URL" in str(exc)
|
||||
else:
|
||||
raise AssertionError("production configuration must reject placeholders")
|
||||
Reference in New Issue
Block a user