Merge pull request #646 from shaun0927/fix/oidc-colon-sub-binding

fix(auth): handle ':' in OIDC raw-username binding placeholders

backend/package/yuxi/services/oidc_service.py

@@ -409,7 +409,11 @@ class OIDCUtils:
         }
 
 
-async def get_or_create_oidc_department(db, dept_name_from_oidc: str | None = None, dept_desc_from_oidc: str | None = None) -> Department | None:
+async def get_or_create_oidc_department(
+    db,
+    dept_name_from_oidc: str | None = None,
+    dept_desc_from_oidc: str | None = None,
+) -> Department | None:
     """获取或创建 OIDC 用户的部门"""
     # 清理并验证从 OIDC 获取的部门名称
     processed_dept_name = None
@@ -494,17 +498,14 @@ async def find_user_by_oidc_sub(db, sub: str) -> User | None:
             if placeholder.is_deleted != 1:
                 # 非deleted占位，直接返回
                 return placeholder
-            parts = placeholder.user_id.split(":")
-            if len(parts) >= 3:
-                try:
-                    target_user_id = int(parts[2])
-                    result = await db.execute(select(User).filter(User.id == target_user_id, User.is_deleted == 0))
-                    target_user = result.scalar_one_or_none()
-                    if target_user:
-                        logger.debug(f"Resolved OIDC binding placeholder {placeholder.user_id} to user {target_user_id}")
-                        return target_user
-                except ValueError:
-                    continue
+            target_user_id = _extract_oidc_placeholder_target_user_id(placeholder.user_id)
+            if target_user_id is None:
+                continue
+            result = await db.execute(select(User).filter(User.id == target_user_id, User.is_deleted == 0))
+            target_user = result.scalar_one_or_none()
+            if target_user:
+                logger.debug(f"Resolved OIDC binding placeholder {placeholder.user_id} to user {target_user_id}")
+                return target_user
         # 如果没有解析出有效的目标用户，返回第一个非deleted legacy用户（向后兼容）
         for candidate in legacy_users:
             if candidate.is_deleted == 0:
@@ -535,20 +536,31 @@ async def find_deleted_oidc_user_by_sub(db, sub: str) -> User | None:
     legacy_users = list(legacy_result.scalars().all())
     if legacy_users:
         for placeholder in legacy_users:
-            parts = placeholder.user_id.split(":")
-            if len(parts) >= 3:
-                try:
-                    target_user_id = int(parts[2])
-                    result = await db.execute(select(User).filter(User.id == target_user_id, User.is_deleted == 1))
-                    target_user = result.scalar_one_or_none()
-                    if target_user:
-                        return target_user
-                except ValueError:
-                    continue
+            target_user_id = _extract_oidc_placeholder_target_user_id(placeholder.user_id)
+            if target_user_id is None:
+                continue
+            result = await db.execute(select(User).filter(User.id == target_user_id, User.is_deleted == 1))
+            target_user = result.scalar_one_or_none()
+            if target_user:
+                return target_user
         return legacy_users[0]
     return None
 
 
+def _extract_oidc_placeholder_target_user_id(user_id: str) -> int | None:
+    """从占位用户ID中解析真实用户ID，允许 sub 中包含冒号。"""
+    value = str(user_id or "").strip()
+    if not value.startswith("oidc:"):
+        return None
+
+    # 占位格式始终以 `:{target_user_id}` 结尾，因此从右侧拆分即可避免 sub 中的冒号干扰。
+    try:
+        _prefix, target_user_id = value.rsplit(":", 1)
+        return int(target_user_id)
+    except ValueError:
+        return None
+
+
 async def _create_oidc_binding_placeholder(db, sub: str, target_user: User) -> None:
     """创建 OIDC sub 绑定占位用户（仅用于记录绑定关系，不用于登录）
 
@@ -591,7 +603,10 @@ async def _create_oidc_binding_placeholder(db, sub: str, target_user: User) -> N
     try:
         db.add(placeholder_user)
         await db.commit()
-        logger.info(f"Created OIDC binding placeholder (deleted) for sub {sub} -> user {target_user.id} ({target_user.user_id})")
+        logger.info(
+            f"Created OIDC binding placeholder (deleted) for sub {sub} -> "
+            f"user {target_user.id} ({target_user.user_id})"
+        )
     except IntegrityError:
         # 并发创建冲突，回滚后忽略
         await db.rollback()
@@ -645,7 +660,10 @@ async def create_oidc_user(db, user_info: dict, department_id: int | None = None
             user_by_sub = await find_user_by_oidc_sub(db, sub)
             if user_by_sub and user_by_sub.id == existing_user.id:
                 # sub 已经正确绑定到该用户，允许返回
-                logger.info(f"User with raw username {user_id} already exists and bound to sub {sub}, returning existing user")
+                logger.info(
+                    f"User with raw username {user_id} already exists and "
+                    f"bound to sub {sub}, returning existing user"
+                )
                 return existing_user
             elif user_by_sub is None:
                 # sub 尚未绑定任何用户，可以将sub绑定到这个现有用户
@@ -654,7 +672,10 @@ async def create_oidc_user(db, user_info: dict, department_id: int | None = None
                 return existing_user
             else:
                 # sub 已经绑定到另一个用户，冲突，拒绝创建
-                logger.warning(f"Cannot create OIDC user with raw username {user_id}: sub {sub} is already bound to another user {user_by_sub.id}, conflict")
+                logger.warning(
+                    f"Cannot create OIDC user with raw username {user_id}: "
+                    f"sub {sub} is already bound to another user {user_by_sub.id}, conflict"
+                )
                 raise HTTPException(
                     status_code=status.HTTP_409_CONFLICT,
                     detail=f"用户名 {user_id} 已存在且OIDC标识 {sub} 已绑定到其他账号，请联系管理员处理冲突",
@@ -797,7 +818,10 @@ async def oidc_callback_handler(code: str, state: str, db, request: Request | No
                 else:
                     # sub 已经绑定到另一个用户，存在冲突，拒绝登录
                     conflict_name = user_by_sub.username if not user_by_name else user_by_name.username
-                    logger.warning(f"OIDC sub {sub} is already bound to a different user, login rejected to prevent account hijacking (conflict: {conflict_name})")
+                    logger.warning(
+                        f"OIDC sub {sub} is already bound to a different user, "
+                        f"login rejected to prevent account hijacking (conflict: {conflict_name})"
+                    )
                     return _redirect_to_login_with_error("OIDC标识已绑定到其他账号，请联系管理员处理绑定冲突")
             else:
                 # sub 尚未绑定到任何用户

backend/test/unit/services/test_oidc_service.py

@@ -0,0 +1,103 @@
+from __future__ import annotations
+
+import os
+from urllib.parse import unquote
+
+import pytest
+import pytest_asyncio
+from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine
+
+os.environ.setdefault("OPENAI_API_KEY", "dummy")
+
+from yuxi.services import oidc_service
+from yuxi.storage.postgres.models_business import User
+
+
+pytestmark = [pytest.mark.asyncio, pytest.mark.unit]
+
+
+@pytest_asyncio.fixture
+async def oidc_session():
+    engine = create_async_engine("sqlite+aiosqlite:///:memory:")
+    async with engine.begin() as conn:
+        await conn.run_sync(User.__table__.create)
+
+    session_factory = async_sessionmaker(engine, expire_on_commit=False)
+    async with session_factory() as session:
+        yield session
+
+    await engine.dispose()
+
+
+async def _create_user(session, user_id: str = "alice") -> User:
+    user = User(username="alice", user_id=user_id, password_hash="x", role="user", is_deleted=0)
+    session.add(user)
+    await session.commit()
+    await session.refresh(user)
+    return user
+
+
+async def test_find_user_by_oidc_sub_resolves_placeholder_when_sub_contains_colon(oidc_session):
+    user = await _create_user(oidc_session)
+
+    await oidc_service._create_oidc_binding_placeholder(oidc_session, "tenant:user", user)
+
+    resolved = await oidc_service.find_user_by_oidc_sub(oidc_session, "tenant:user")
+
+    assert resolved is not None
+    assert resolved.id == user.id
+    assert resolved.user_id == user.user_id
+    assert resolved.is_deleted == 0
+
+
+async def test_find_deleted_oidc_user_by_sub_resolves_deleted_target_when_sub_contains_colon(oidc_session):
+    user = await _create_user(oidc_session)
+    user.is_deleted = 1
+    await oidc_session.commit()
+
+    await oidc_service._create_oidc_binding_placeholder(oidc_session, "tenant:user", user)
+
+    resolved = await oidc_service.find_deleted_oidc_user_by_sub(oidc_session, "tenant:user")
+
+    assert resolved is not None
+    assert resolved.id == user.id
+    assert resolved.user_id == user.user_id
+    assert resolved.is_deleted == 1
+
+
+async def test_oidc_callback_allows_existing_binding_when_sub_contains_colon(oidc_session, monkeypatch):
+    user = await _create_user(oidc_session)
+    await oidc_service._create_oidc_binding_placeholder(oidc_session, "tenant:user", user)
+
+    monkeypatch.setattr(oidc_service.oidc_config, "enabled", True)
+    monkeypatch.setattr(oidc_service.oidc_config, "client_id", "cid")
+    monkeypatch.setattr(oidc_service.oidc_config, "client_secret", "secret")
+    monkeypatch.setattr(oidc_service.oidc_config, "token_endpoint", "https://example/token")
+    monkeypatch.setattr(oidc_service.oidc_config, "authorization_endpoint", "https://example/auth")
+    monkeypatch.setattr(oidc_service.oidc_config, "userinfo_endpoint", "https://example/userinfo")
+    monkeypatch.setattr(oidc_service.oidc_config, "use_raw_username", True)
+    monkeypatch.setattr(oidc_service.oidc_config, "auto_create_user", False)
+
+    monkeypatch.setattr(
+        oidc_service.OIDCUtils,
+        "verify_state",
+        classmethod(lambda cls, state: {"redirect_path": "/"}),
+    )
+
+    async def fake_exchange(cls, code):
+        return {"access_token": "token"}
+
+    async def fake_userinfo(cls, access_token):
+        return {"sub": "tenant:user", "preferred_username": "alice"}
+
+    async def fake_log_operation(db, user_id, operation, request=None):
+        return None
+
+    monkeypatch.setattr(oidc_service.OIDCUtils, "exchange_code_for_token", classmethod(fake_exchange))
+    monkeypatch.setattr(oidc_service.OIDCUtils, "get_userinfo", classmethod(fake_userinfo))
+    monkeypatch.setattr(oidc_service, "log_operation", fake_log_operation)
+
+    response = await oidc_service.oidc_callback_handler("dummy-code", "dummy-state", oidc_session)
+
+    assert response.status_code == 302
+    assert unquote(response.headers["location"]).startswith("/auth/oidc/callback?code=")

docs/develop-guides/roadmap.md

@@ -48,6 +48,7 @@
 - 重构 MCP 运行时配置加载模型：移除 `MCP_SERVERS` 作为运行正确性前提的设计，改为每次直接从数据库读取最新 MCP 配置，并用 `server_name:config_hash` 作为本地工具缓存 key；同时将内置 MCP 初始化职责收敛为仅同步数据库默认项，前端 MCP 选项改为直接使用实时资源列表，解决 `api`/`worker` 分进程下的配置不一致与缓存失效问题
 - 为知识库检索工具补充 `metadata.filepath` 注入：在 `query_kb` 统一出口基于会话可见知识库构建 `file_id -> /home/gem/kbs/...` 映射并回填 Milvus 检索结果，注入逻辑复用知识库只读后端命名规则；路径注入仅作用于 Milvus chunks 列表，Dify 和 LightRAG 等其他知识库保持原检索结果返回，不再兼容无显式 `file_id` 的推断注入，新增单测覆盖该约束
 - 调整 Milvus 混合检索实现：集合 schema 增加 Milvus 内置 BM25 稀疏向量字段、BM25 函数和中文 analyzer 配置，`keyword` 模式改为 BM25 全文检索，`hybrid` 模式改为 Milvus 原生向量 + BM25 混合检索，并同步更新检索参数说明。
+- 修复 OIDC 原始用户名绑定中的占位用户解析：绑定占位格式仍保持 `oidc:{sub}:{target_user_id}`，但在解析目标用户 ID 时改为从右侧拆分，避免 `sub` 中包含冒号时把已绑定账号误判成冲突账号，并补充对应单元测试覆盖该回归。
 - 修复 DOCX 解析中的图片回插顺序：Docling 导出的多个 `<!-- image -->` 占位符现在按文档图片顺序替换，避免多图文档中的图片链接前后颠倒。
 - 修复前端依赖安全告警：通过 `pnpm.overrides` 将传递依赖 `flatted` 锁定到 `3.4.2`、`lodash-es` 锁定到 `4.18.1`，并同步更新 `pnpm-lock.yaml` 以消除 DriftGuard 报告的高危 CVE
 - 重写界面设计规范：参考 `DESIGN.md` 写法补充视觉气质、颜色 token、组件状态、布局层级、响应式与 Agent Prompt Guide，并基于该规范收敛首页视觉表现，移除装饰性渐变、重阴影、hover 位移和入场动画。