fix(auth): store passwords with bcrypt and migrate legacy MD5 digests #1432

backend/apps/system/api/login.py

@@ -6,7 +6,7 @@
 from common.core.deps import SessionDep, Trans
 from common.utils.crypto import sqlbot_decrypt
 from ..crud.user import authenticate, check_pwd_format, clean_user_cache, get_db_user
-from common.core.security import create_access_token, default_pwd, md5pwd, verify_md5pwd
+from common.core.security import create_access_token, default_pwd, get_password_hash, verify_password
 from datetime import timedelta
 from common.core.config import settings
 from common.core.schemas import Token
@@ -44,7 +44,7 @@ async def local_login(
     if (
         user.id != 1
         and await initial_pwd_disabled(session)
-        and verify_md5pwd(default_pwd(), user.password)
+        and verify_password(default_pwd(), user.password)
     ):
         return Token(access_token='', need_change_pwd=True)
     access_token_expires = timedelta(minutes=settings.ACCESS_TOKEN_EXPIRE_MINUTES)
@@ -75,7 +75,7 @@ async def login_change_pwd(session: SessionDep, trans: Trans, editor: LoginPwdEd
     if await initial_pwd_disabled(session) and new_pwd == default_pwd():
         raise HTTPException(status_code=400, detail=trans('i18n_login.new_pwd_is_initial'))
     db_user = get_db_user(session=session, user_id=user.id)
-    db_user.password = md5pwd(new_pwd)
+    db_user.password = get_password_hash(new_pwd)
     session.add(db_user)
     await clean_user_cache(user.id)
     return db_user
@@ -84,4 +84,4 @@ async def login_change_pwd(session: SessionDep, trans: Trans, editor: LoginPwdEd
 async def logout(session: SessionDep, request: Request, dto: LogoutSchema):
     if dto.origin != 0:
         return await xpack_logout(session, request, dto)
-    return None
+    return None

backend/apps/system/api/user.py

@@ -15,7 +15,7 @@
 from common.core.deps import CurrentUser, SessionDep, Trans
 from common.core.pagination import Paginator
 from common.core.schemas import PaginatedResponse, PaginationParams
-from common.core.security import default_md5_pwd, md5pwd, verify_md5pwd
+from common.core.security import default_pwd_hash, get_password_hash, verify_password
 from common.core.sqlbot_cache import clear_cache
 from common.core.config import settings
 from apps.swagger.i18n import PLACEHOLDER_PREFIX
@@ -386,7 +386,7 @@ async def pwdReset(session: SessionDep, current_user: CurrentUser, trans: Trans,
     if not current_user.isAdmin:
         raise Exception(trans('i18n_permission.no_permission', url = " patch[/user/pwd/id],", msg = trans('i18n_permission.only_admin')))
     db_user: UserModel = get_db_user(session=session, user_id=id)
-    db_user.password = default_md5_pwd()
+    db_user.password = default_pwd_hash()
     session.add(db_user)
 
 @router.put("/pwd", summary=f"{PLACEHOLDER_PREFIX}update_pwd", description=f"{PLACEHOLDER_PREFIX}update_pwd")
@@ -397,11 +397,11 @@ async def pwdUpdate(session: SessionDep, current_user: CurrentUser, trans: Trans
     if not check_pwd_format(new_pwd):
         raise Exception(trans('i18n_format_invalid', key = trans('i18n_user.password')))
     db_user: UserModel = get_db_user(session=session, user_id=current_user.id)
-    if not verify_md5pwd(editor.pwd, db_user.password):
+    if not verify_password(editor.pwd, db_user.password):
         raise Exception(trans('i18n_error', key = trans('i18n_user.password')))
     if await initial_pwd_disabled(session) and new_pwd == settings.DEFAULT_PWD:
         raise HTTPException(status_code=400, detail=trans('i18n_login.new_pwd_is_initial'))
-    db_user.password = md5pwd(new_pwd)
+    db_user.password = get_password_hash(new_pwd)
     session.add(db_user)
     return db_user
 

backend/apps/system/crud/user.py

@@ -6,7 +6,7 @@
 from apps.system.schemas.auth import CacheName, CacheNamespace
 from apps.system.schemas.system_schema import EMAIL_REGEX, PWD_REGEX, BaseUserDTO, UserInfoDTO, UserWs
 from common.core.deps import SessionDep
-from common.core.security import verify_md5pwd
+from common.core.security import get_password_hash, is_legacy_md5_pwd, verify_password
 from common.core.sqlbot_cache import cache, clear_cache
 from common.utils.locale import I18n, I18nHelper
 from common.utils.utils import SQLBotLogUtil
@@ -45,11 +45,26 @@ def authenticate(*, session: Session, account: str, password: str) -> BaseUserDT
     db_user = get_user_by_account(session=session, account=account)
     if not db_user:
         return None
-    if not verify_md5pwd(password, db_user.password):
+    if not verify_password(password, db_user.password):
         return None
+    upgrade_legacy_pwd_hash(session=session, user_id=db_user.id, password=password)
     return db_user
 
 
+def upgrade_legacy_pwd_hash(*, session: Session, user_id: int, password: str) -> None:
+    """Rewrite a just-verified legacy MD5 hash with bcrypt (issue #1432).
+
+    Only reached after `verify_password` accepted `password`, so the new hash is
+    derived from the same plaintext. The request-scoped session commits at the
+    end of the call (common.core.db.get_session).
+    """
+    db_user: UserModel = get_db_user(session=session, user_id=user_id)
+    if not db_user or not is_legacy_md5_pwd(db_user.password):
+        return
+    db_user.password = get_password_hash(password)
+    session.add(db_user)
+
+
 def user_ws_list(session: Session, uid: int, trans: Optional[I18n | I18nHelper] = None) -> list[UserWs]:
     if uid == 1:
         stmt = select(WorkspaceModel.id, WorkspaceModel.name).order_by(WorkspaceModel.name, WorkspaceModel.create_time)

backend/apps/system/models/user.py

@@ -5,15 +5,15 @@
 from sqlmodel import SQLModel, Field
 
 from common.core.models import SnowflakeBase
-from common.core.security import default_md5_pwd
+from common.core.security import default_pwd_hash
 from common.utils.time import get_timestamp
 
 
 class BaseUserPO(SQLModel):
     account: str = Field(max_length=255, unique=True)
     oid: int = Field(nullable=False, sa_type=BigInteger(), default=0)
     name: str = Field(max_length=255, unique=True)
-    password: str = Field(default_factory=default_md5_pwd, max_length=255)
+    password: str = Field(default_factory=default_pwd_hash, max_length=255)
     email: str = Field(max_length=255)
     status: int = Field(default=0, nullable=False)
     origin: int = Field(nullable=False, default=0)

backend/common/core/security.py

@@ -1,16 +1,24 @@
+import hashlib
+import hmac
+import re
 from datetime import datetime, timedelta, timezone
 from typing import Any
 
 import jwt
 from passlib.context import CryptContext
-import hashlib
+
 from common.core.config import settings
 
 pwd_context = CryptContext(schemes=["bcrypt"], deprecated="auto")
 
 
 ALGORITHM = "HS256"
 
+# Issue #1432: passwords used to be stored as unsalted MD5 digests. Rows that
+# predate the bcrypt migration still carry one of these; they are accepted for
+# verification only and are rewritten as bcrypt on the next successful login.
+LEGACY_MD5_PWD_PATTERN = re.compile(r"^[0-9a-fA-F]{32}$")
+
 
 def create_access_token(data: dict | Any, expires_delta: timedelta) -> str:
     to_encode = data.copy()
@@ -21,24 +29,69 @@ def create_access_token(data: dict | Any, expires_delta: timedelta) -> str:
     return encoded_jwt
 
 
+def get_password_hash(password: str) -> str:
+    """Hash a password for storage: bcrypt, never an unsalted MD5 digest (issue #1432)."""
+    return pwd_context.hash(password)
+
+
+def is_legacy_md5_pwd(hashed_password: str) -> bool:
+    """True when a stored value is a legacy unsalted MD5 digest instead of a bcrypt hash."""
+    return (
+        bool(hashed_password)
+        and LEGACY_MD5_PWD_PATTERN.fullmatch(hashed_password) is not None
+    )
+
+
 def verify_password(plain_password: str, hashed_password: str) -> bool:
-    return pwd_context.verify(plain_password, hashed_password)
+    """Verify a password against a stored hash.
 
+    bcrypt hashes are checked with passlib. Legacy unsalted MD5 digests stay
+    accepted so deployments created before issue #1432 was fixed can still log
+    in; the caller that owns the session rewrites them with bcrypt right
+    afterwards (see `apps.system.crud.user.authenticate`).
+    """
+    if not plain_password or not hashed_password:
+        return False
+    if is_legacy_md5_pwd(hashed_password):
+        return hmac.compare_digest(md5pwd(plain_password), hashed_password.lower())
+    try:
+        return pwd_context.verify(plain_password, hashed_password)
+    except (ValueError, TypeError):
+        return False
 
-def get_password_hash(password: str) -> str:
-    return pwd_context.hash(password)
 
 def md5pwd(password: str) -> str:
+    """Legacy unsalted MD5 digest (issue #1432).
+
+    Kept only so rows stored before the migration stay verifiable and can be
+    upgraded. Never use it to store a new or changed password.
+    """
     m = hashlib.md5()
     m.update(password.encode("utf-8"))
     return m.hexdigest()
 
+
 def verify_md5pwd(plain_password: str, md5_password: str) -> bool:
-    return md5pwd(plain_password) == md5_password
+    """Legacy-only verification against an unsalted MD5 digest (issue #1432)."""
+    if not plain_password or not md5_password:
+        return False
+    return hmac.compare_digest(md5pwd(plain_password), md5_password.lower())
+
 
 def default_pwd() -> str:
     return settings.DEFAULT_PWD
 
+
+def default_pwd_hash() -> str:
+    """bcrypt hash of the configured default password, used when resetting a user (issue #1432)."""
+    return get_password_hash(default_pwd())
+
+
 def default_md5_pwd() -> str:
+    """Legacy digest of the default password (issue #1432).
+
+    Kept so existing imports keep resolving; new call sites must use
+    `default_pwd_hash` instead.
+    """
     pwd = default_pwd()
-    return md5pwd(pwd)
+    return md5pwd(pwd)

backend/tests/test_initial_password_policy.py

@@ -2,11 +2,15 @@
 
 Extract actual functions to avoid importing DB/xpack startup. These tests do not
 claim coverage of authentication middleware, decorators or real transactions.
+
+Guard update (issue #1432): stored passwords moved from unsalted MD5 digests to
+bcrypt, so the stubs now follow `get_password_hash` / `verify_password`. The
+assertions still pin what this guard always protected: password state only
+changes when the old credential and the new format are both accepted.
 """
 
 import ast
 import asyncio
-import hashlib
 import json
 from pathlib import Path
 from types import SimpleNamespace
@@ -16,6 +20,7 @@
 from fastapi import HTTPException
 
 ROOT = Path(__file__).resolve().parents[2]
+BCRYPT_STUB_PREFIX = "bcrypt-stub$"
 
 
 def load_function(path, name, namespace):
@@ -40,13 +45,14 @@ def load_function(path, name, namespace):
     return namespace[name]
 
 
-def md5(value):
-    return hashlib.md5(value.encode()).hexdigest()
+def bcrypt_stub(value):
+    """Deterministic stand-in for `common.core.security.get_password_hash`."""
+    return f"{BCRYPT_STUB_PREFIX}{value}"
 
 
 @pytest.fixture
 def context():
-    user = SimpleNamespace(id=2, password=md5("OldPassword42!"))
+    user = SimpleNamespace(id=2, password=bcrypt_stub("OldPassword42!"))
     session = Mock()
     args = AsyncMock(
         return_value=[SimpleNamespace(pkey="login.initial_pwd_disabled", pval="true")]
@@ -55,8 +61,8 @@ def context():
         "get_group_args": args,
         "HTTPException": HTTPException,
         "get_db_user": Mock(return_value=user),
-        "md5pwd": md5,
-        "verify_md5pwd": lambda value, hashed: md5(value) == hashed,
+        "get_password_hash": bcrypt_stub,
+        "verify_password": lambda value, hashed: hashed == bcrypt_stub(value),
         "default_pwd": lambda: "InitialPassword42!",
         "settings": SimpleNamespace(DEFAULT_PWD="InitialPassword42!"),
     }
@@ -115,14 +121,14 @@ def test_disabled_off_allows_initial_password(context, value):
     user, session, args, _, _ = context
     args.return_value = [SimpleNamespace(pkey="login.initial_pwd_disabled", pval=value)]
     update(context)
-    assert user.password == md5("InitialPassword42!")
+    assert user.password == bcrypt_stub("InitialPassword42!")
     session.add.assert_called_once_with(user)
 
 
 def test_enabled_allows_other_password(context):
     user, session, _, _, _ = context
     update(context, new="DifferentPassword42!")
-    assert user.password == md5("DifferentPassword42!")
+    assert user.password == bcrypt_stub("DifferentPassword42!")
     session.add.assert_called_once_with(user)
 
 

backend/tests/test_mcp_parameters_integration.py

@@ -31,7 +31,7 @@
 
 from apps.system.models.system_model import AiModelDetail, UserWsModel
 from apps.system.models.user import UserModel
-from common.core.security import md5pwd
+from common.core.security import get_password_hash
 
 table_environment = table_contracts.table_environment
 
@@ -139,7 +139,7 @@ def test_mcp_network_task_uses_parameter_saved_by_another_api_process(
                 name="MCP test",
                 oid=1,
                 email="mcp@example.invalid",
-                password=md5pwd(password),
+                password=get_password_hash(password),
                 status=1,
             )
         )