fix: vá lỗ hổng RBAC + hardcode sàn ở /orders/place, nâng cấp mã hoá và CORS
- orders.place_order: yêu cầu role trader/admin (get_current_trader_user) thay vì bất kỳ user đã đăng nhập nào — viewer không còn đặt được lệnh thật - orders.place_order: resolve exchange theo OrderRequest.exchange thay vì hardcode "mexc", fallback về credential active gần nhất nếu không truyền - security.py: mã hoá API key chuyển AES-256-CBC -> AES-256-GCM (có xác thực toàn vẹn), giữ đường giải mã cũ để credential đã lưu trước đây không hỏng - main_api.py: CORS_ORIGINS rỗng -> deny-all thay vì fallback "*" (kèm allow_credentials=True là cấu hình nguy hiểm) - docker-compose.yml: đồng bộ DB_PASSWORD giữa backend-api/scheduler và db - frontend: OrderPanel/DashboardPage truyền kèm exchange đang chọn khi đặt lệnh Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Le
@@ -9,7 +9,7 @@ from fastapi import APIRouter, Depends, status
|
||||
from sqlalchemy import select
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from app.core.deps import get_current_user, get_db_session
|
||||
from app.core.deps import get_current_trader_user, get_current_user, get_db_session
|
||||
from app.core.exceptions import NotFoundException, ValidationException
|
||||
from app.core.security import decrypt_api_key
|
||||
from app.exchange.factory import factory as exchange_factory
|
||||
@@ -26,12 +26,13 @@ router = APIRouter(prefix="/orders", tags=["orders"])
|
||||
async def place_order(
|
||||
req: OrderRequest,
|
||||
db: AsyncSession = Depends(get_db_session),
|
||||
current_user: User = Depends(get_current_user),
|
||||
current_user: User = Depends(get_current_trader_user),
|
||||
) -> OrderData:
|
||||
"""Place an order on a connected exchange.
|
||||
|
||||
Uses the user's saved API credentials for the exchange.
|
||||
The exchange name is inferred from the symbol or passed explicitly.
|
||||
Uses the user's saved API credentials for the exchange. If ``req.exchange``
|
||||
is given, that exchange's credential is used; otherwise falls back to the
|
||||
user's only (or first) active credential.
|
||||
"""
|
||||
# Currently only market orders are supported via this endpoint
|
||||
if req.order_type not in ("market", "limit"):
|
||||
@@ -41,27 +42,33 @@ async def place_order(
|
||||
if req.amount <= 0:
|
||||
raise ValidationException(detail="Amount must be positive")
|
||||
|
||||
# Infer exchange name from symbol prefix heuristics, or default to mexc
|
||||
# In a more advanced setup the user would specify exchange_id in the request
|
||||
exchange_name = "mexc"
|
||||
|
||||
# Find the user's active credential for this exchange
|
||||
result = await db.execute(
|
||||
# Find the user's active credential — filter by requested exchange if given,
|
||||
# otherwise fall back to whichever active credential the user has.
|
||||
query = (
|
||||
select(ExchangeCredential)
|
||||
.join(Exchange, Exchange.id == ExchangeCredential.exchange_id)
|
||||
.where(
|
||||
ExchangeCredential.user_id == current_user.id,
|
||||
Exchange.name == exchange_name,
|
||||
ExchangeCredential.is_active == True,
|
||||
)
|
||||
)
|
||||
cred = result.scalar_one_or_none()
|
||||
if req.exchange:
|
||||
query = query.where(Exchange.name == req.exchange.lower())
|
||||
else:
|
||||
query = query.order_by(ExchangeCredential.created_at.desc())
|
||||
|
||||
result = await db.execute(query)
|
||||
cred = result.scalars().first()
|
||||
if cred is None:
|
||||
target = req.exchange or "any exchange"
|
||||
raise NotFoundException(
|
||||
detail=f"No active API key found for {exchange_name}. "
|
||||
detail=f"No active API key found for {target}. "
|
||||
f"Go to Profile → API Keys to add one."
|
||||
)
|
||||
|
||||
exchange_result = await db.execute(select(Exchange).where(Exchange.id == cred.exchange_id))
|
||||
exchange_name = exchange_result.scalar_one().name
|
||||
|
||||
# Decrypt the stored API key/secret
|
||||
try:
|
||||
# api_key is stored as plaintext (masked in responses), api_secret is encrypted
|
||||
|
||||
@@ -16,7 +16,9 @@ from fastapi import HTTPException
|
||||
from jose import JWTError, jwt
|
||||
from jose.exceptions import ExpiredSignatureError
|
||||
from passlib.context import CryptContext
|
||||
from cryptography.exceptions import InvalidTag
|
||||
from cryptography.hazmat.primitives.ciphers import Cipher, algorithms, modes
|
||||
from cryptography.hazmat.primitives.ciphers.aead import AESGCM
|
||||
from cryptography.hazmat.backends import default_backend
|
||||
|
||||
from app.config import settings
|
||||
@@ -259,11 +261,22 @@ def decode_token(token: str) -> dict:
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# AES-256-CBC encryption (for API key storage)
|
||||
# AES encryption (for API key storage)
|
||||
#
|
||||
# Current scheme: AES-256-GCM (authenticated encryption — tamper-evident).
|
||||
# Legacy scheme: AES-256-CBC (no integrity check), kept read-only so API
|
||||
# keys encrypted before this migration can still be decrypted.
|
||||
#
|
||||
# The two schemes are told apart by the stored IV/nonce length: GCM nonces
|
||||
# are 12 bytes (24 hex chars), legacy CBC IVs are 16 bytes (32 hex chars).
|
||||
# New/updated credentials are always re-encrypted with GCM going forward.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
_BACKEND = default_backend()
|
||||
|
||||
_GCM_NONCE_LENGTH = 12 # bytes
|
||||
_CBC_IV_LENGTH = 16 # bytes
|
||||
|
||||
|
||||
def generate_encryption_key() -> str:
|
||||
"""Generate a random 32-byte (256-bit) hex-encoded encryption key.
|
||||
@@ -290,22 +303,17 @@ def encrypt_api_key(
|
||||
api_key: str,
|
||||
key_hex: Optional[str] = None,
|
||||
) -> Tuple[str, str]:
|
||||
"""Encrypt an API key with AES-256-CBC.
|
||||
"""Encrypt an API key with AES-256-GCM (authenticated encryption).
|
||||
|
||||
Returns ``(ciphertext_hex, iv_hex)``.
|
||||
Returns ``(ciphertext_hex, nonce_hex)``. The returned ciphertext includes
|
||||
the 16-byte GCM authentication tag appended by the library, so a
|
||||
corrupted/tampered value fails to decrypt instead of silently returning
|
||||
garbage plaintext (as plain CBC would).
|
||||
"""
|
||||
key = _resolve_key(key_hex)
|
||||
iv = uuid.uuid4().bytes # 16 random bytes
|
||||
cipher = Cipher(algorithms.AES(key), modes.CBC(iv), backend=_BACKEND)
|
||||
encryptor = cipher.encryptor()
|
||||
|
||||
# Pad plaintext to AES block size (16 bytes) using PKCS7
|
||||
plaintext_bytes = api_key.encode("utf-8")
|
||||
pad_len = 16 - (len(plaintext_bytes) % 16)
|
||||
padded = plaintext_bytes + bytes([pad_len] * pad_len)
|
||||
|
||||
ciphertext = encryptor.update(padded) + encryptor.finalize()
|
||||
return ciphertext.hex(), iv.hex()
|
||||
nonce = uuid.uuid4().bytes[:_GCM_NONCE_LENGTH] # 12 random bytes
|
||||
ciphertext = AESGCM(key).encrypt(nonce, api_key.encode("utf-8"), None)
|
||||
return ciphertext.hex(), nonce.hex()
|
||||
|
||||
|
||||
def decrypt_api_key(
|
||||
@@ -313,14 +321,38 @@ def decrypt_api_key(
|
||||
iv_hex: str,
|
||||
key_hex: Optional[str] = None,
|
||||
) -> str:
|
||||
"""Decrypt an AES-256-CBC encrypted API key.
|
||||
"""Decrypt an API key encrypted with either scheme above.
|
||||
|
||||
Returns the original plaintext string.
|
||||
Dispatches on the stored IV/nonce length so credentials encrypted before
|
||||
the AES-GCM migration keep working without a data migration.
|
||||
"""
|
||||
key = _resolve_key(key_hex)
|
||||
ciphertext = bytes.fromhex(ciphertext_hex)
|
||||
iv = bytes.fromhex(iv_hex)
|
||||
|
||||
if len(iv) == _GCM_NONCE_LENGTH:
|
||||
ciphertext = bytes.fromhex(ciphertext_hex)
|
||||
try:
|
||||
plaintext = AESGCM(key).decrypt(iv, ciphertext, None)
|
||||
except InvalidTag:
|
||||
raise HTTPException(
|
||||
status_code=500,
|
||||
detail="Decryption failed: authentication tag mismatch (wrong key or corrupted data)",
|
||||
)
|
||||
return plaintext.decode("utf-8")
|
||||
|
||||
if len(iv) == _CBC_IV_LENGTH:
|
||||
return _decrypt_api_key_cbc_legacy(ciphertext_hex, iv, key)
|
||||
|
||||
raise HTTPException(
|
||||
status_code=500,
|
||||
detail=f"Decryption failed: unrecognized IV length ({len(iv)} bytes)",
|
||||
)
|
||||
|
||||
|
||||
def _decrypt_api_key_cbc_legacy(ciphertext_hex: str, iv: bytes, key: bytes) -> str:
|
||||
"""Decrypt a pre-migration AES-256-CBC ciphertext. Read-only legacy path."""
|
||||
ciphertext = bytes.fromhex(ciphertext_hex)
|
||||
|
||||
cipher = Cipher(algorithms.AES(key), modes.CBC(iv), backend=_BACKEND)
|
||||
decryptor = cipher.decryptor()
|
||||
padded = decryptor.update(ciphertext) + decryptor.finalize()
|
||||
|
||||
@@ -58,6 +58,7 @@ class OrderRequest(BaseModel):
|
||||
price: Optional[Decimal] = None # required for limit orders
|
||||
reduce_only: bool = False
|
||||
position_side: Optional[str] = None # "long" or "short" (for futures)
|
||||
exchange: Optional[str] = None # target exchange name; defaults to user's active credential
|
||||
|
||||
|
||||
class OrderData(BaseModel):
|
||||
|
||||
@@ -82,11 +82,18 @@ app = FastAPI(
|
||||
lifespan=lifespan,
|
||||
)
|
||||
|
||||
# CORS
|
||||
# CORS — fail closed: an unset/empty CORS_ORIGINS must never fall back to "*"
|
||||
# (a wildcard origin combined with allow_credentials=True is a real browser
|
||||
# exploitation vector, not just a lint warning). Deny all cross-origin
|
||||
# requests by default and log loudly so the misconfiguration is obvious.
|
||||
if settings.CORS_ORIGINS:
|
||||
origins = [o.strip() for o in settings.CORS_ORIGINS.split(",") if o.strip()]
|
||||
else:
|
||||
origins = ["*"]
|
||||
origins = []
|
||||
logger.warning(
|
||||
"CORS_ORIGINS is not set — denying all cross-origin requests by default. "
|
||||
"Set CORS_ORIGINS in the environment to allow your frontend's origin(s)."
|
||||
)
|
||||
|
||||
app.add_middleware(CORSMiddleware, allow_origins=origins, allow_credentials=True,
|
||||
allow_methods=["*"], allow_headers=["*"])
|
||||
|
||||
+2
-2
@@ -37,7 +37,7 @@ services:
|
||||
db:
|
||||
condition: service_healthy
|
||||
environment:
|
||||
DATABASE_URL: postgresql+asyncpg://trading:trading_secret@db:5432/trading_portal
|
||||
DATABASE_URL: postgresql+asyncpg://trading:${DB_PASSWORD:-trading_secret}@db:5432/trading_portal
|
||||
PORT: 8001
|
||||
ENCRYPTION_KEY: ${ENCRYPTION_KEY}
|
||||
CORS_ORIGINS: http://localhost,http://localhost:5173,http://localhost:3000
|
||||
@@ -76,7 +76,7 @@ services:
|
||||
db:
|
||||
condition: service_healthy
|
||||
environment:
|
||||
DATABASE_URL: postgresql+asyncpg://trading:trading_secret@db:5432/trading_portal
|
||||
DATABASE_URL: postgresql+asyncpg://trading:${DB_PASSWORD:-trading_secret}@db:5432/trading_portal
|
||||
ENCRYPTION_KEY: ${ENCRYPTION_KEY}
|
||||
LOG_LEVEL: INFO
|
||||
JWT_PRIVATE_KEY_PATH: /run/secrets/jwt_private.pem
|
||||
|
||||
@@ -348,6 +348,7 @@ export default function DashboardPage() {
|
||||
) : (
|
||||
<OrderPanel
|
||||
symbol={symbol || '—'}
|
||||
exchange={exchange}
|
||||
lastPrice={lastPrice}
|
||||
balance={balance}
|
||||
isConnected={balance !== null}
|
||||
|
||||
@@ -4,6 +4,7 @@ import { useAppSelector } from '../../app/hooks';
|
||||
|
||||
interface Props {
|
||||
symbol: string;
|
||||
exchange: string;
|
||||
lastPrice: number | null;
|
||||
balance: {asset: string; free: number; used: number; total: number}[] | null;
|
||||
isConnected: boolean;
|
||||
@@ -23,7 +24,7 @@ function formatPrice(price: number): string {
|
||||
return '$' + price.toLocaleString(undefined, { minimumFractionDigits: 2, maximumFractionDigits: 2 });
|
||||
}
|
||||
|
||||
export default function OrderPanel({ symbol, lastPrice, balance, isConnected }: Props) {
|
||||
export default function OrderPanel({ symbol, exchange, lastPrice, balance, isConnected }: Props) {
|
||||
const { user } = useAppSelector((state) => state.auth);
|
||||
const defaultTradeSize = user?.preferences?.trade_size || 10;
|
||||
const [currentTime, setCurrentTime] = useState(new Date());
|
||||
@@ -97,6 +98,7 @@ export default function OrderPanel({ symbol, lastPrice, balance, isConnected }:
|
||||
method: 'POST',
|
||||
body: JSON.stringify({
|
||||
symbol,
|
||||
exchange,
|
||||
side,
|
||||
order_type: 'market',
|
||||
amount: amt,
|
||||
|
||||
Reference in New Issue
Block a user