Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 23 additions & 19 deletions apps/common/middleware/chat_headers_middleware.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
# coding=utf-8
"""
@project: maxkb
@Author:虎
@file: static_headers_middleware.py
@date:2024/3/13 18:26
@desc:
@project: maxkb
@Author:虎
@file: static_headers_middleware.py
@date:2024/3/13 18:26
@desc:
"""

from django.utils.deprecation import MiddlewareMixin

from common.cache_data.application_access_token_cache import get_application_access_token
Expand All @@ -17,23 +18,26 @@ class ChatHeadersMiddleware(MiddlewareMixin):
def process_response(self, request, response):

if request.path.startswith(CONFIG.get_chat_path()) and not request.path.startswith(
CONFIG.get_chat_path() + '/api'):
access_token = request.path.replace(CONFIG.get_chat_path() + '/', '')
if access_token.__contains__('/') or access_token == 'undefined':
CONFIG.get_chat_path() + "/api"
):
access_token = request.path.replace(CONFIG.get_chat_path() + "/", "")
if access_token.__contains__("/") or access_token == "undefined":
return response
application_access_token = get_application_access_token(access_token, True)
if application_access_token is not None:
white_active = application_access_token.get('white_active', False)
white_list = application_access_token.get('white_list', [])
application_icon = escape(application_access_token.get('application_icon') or '')
application_name = escape(application_access_token.get('application_name') or '')
white_active = application_access_token.get("white_active", False)
white_list = application_access_token.get("white_list", [])
application_icon = escape(application_access_token.get("application_icon") or "")
application_name = escape(application_access_token.get("application_name") or "")
if white_active:
# 添加自定义的响应头
response[
'Content-Security-Policy'] = f'frame-ancestors {" ".join(white_list)}'
response.content = (response.content.decode('utf-8').replace(
'<link rel="icon" href="./favicon.ico"/>',
f'<link rel="icon" href="{application_icon}" />')
.replace('<title>MaxKB</title>', f'<title>{application_name}</title>').encode(
"utf-8"))
response["Content-Security-Policy"] = f"frame-ancestors {' '.join(white_list)}"
response.content = (
response.content.decode("utf-8")
.replace(
'<link rel="icon" href="./favicon.ico" />', f'<link rel="icon" href="{application_icon}" />'
)
.replace("<title>MaxKB</title>", f"<title>{application_name}</title>")
.encode("utf-8")
)
return response

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's my review of the diff:

Potential Issues

  1. Silent behavior change in replace() (BUG)

    • Original: '<link rel="icon" href="./favicon.ico"/>' (no space before />)
    • New: '<link rel="icon" href="./favicon.ico" />' (space added)

    This is a meaningful change, not just formatting. If the actual HTML template contains ...favicon.ico"/> without the space, the str.replace() will silently do nothing and the favicon won't be replaced. Whatever changed the string content should be treated as a functional change and verified against the real template. Revert if unintended.

  2. escape() output inserted into HTML attributes

    • escape() (Django's django.utils.html.escape) escapes < > & ' ", which is reasonable for attribute context. But application_icon is placed inside an href="...". Consider validating it's a safe URL scheme (e.g., reject javascript:), since escaping doesn't prevent scheme-based XSS on older browsers.
  3. white_list used unvalidated in CSP header

    • " ".join(white_list) assumes a list of strings. If any entry is user-controlled, a value containing spaces/tokens could inject extra CSP directives. Validate each origin. Also, if white_list is empty while white_active is True, you emit frame-ancestors (empty), which may block all framing — worth confirming that's intended.
  4. access_token.__contains__("/") — works, but "/" in access_token is more idiomatic and faster. Same for == "undefined" check; consider also handling empty string after strip.

  5. response.content.decode("utf-8") on non-HTML/binary responses

    • The path match (startswith(chat_path), not /api) may still hit static assets (JS/CSS/images). Decoding arbitrary bytes as UTF-8 can raise UnicodeDecodeError, crashing the response. Guard on Content-Type starting with text/html before decoding/replacing.
  6. None vs missing check for application_access_token

    • Fine as-is, but note get_application_access_token(..., True) semantics (cached=True?) — confirm caching TTL doesn't cause stale icon/name.

Style / Minor

  • Mixed quoting: the file now uses "" everywhere, but the module docstring header keys (@project:, etc.) still use full-width colon : — cosmetic.
  • The comment # 添加自定义的响应头 is Chinese; if the codebase standardizes on English comments, translate it.
  • The multi-line .replace(...) chain is fine, but wrapping CONFIG.get_chat_path() in a local variable would avoid repeated calls and improve readability:
    chat_path = CONFIG.get_chat_path()
    if request.path.startswith(chat_path) and not request.path.startswith(chat_path + "/api"):
        ...

Suggested Rewrite (key parts)

chat_path = CONFIG.get_chat_path()
if request.path.startswith(chat_path) and not request.path.startswith(chat_path + "/api"):
    access_token = request.path.replace(chat_path + "/", "")
    if "/" in access_token or access_token == "undefined" or not access_token:
        return response

    token_info = get_application_access_token(access_token, True)
    if token_info is None:
        return response

    if response.get("Content-Type", "").split(";")[0].strip() != "text/html":
        return response

    white_active = token_info.get("white_active", False)
    white_list = token_info.get("white_list", []) or []
    application_icon = escape(token_info.get("application_icon") or "")
    application_name = escape(token_info.get("application_name") or "")

    if white_active and white_list:
        response["Content-Security-Policy"] = f"frame-ancestors {' '.join(white_list)}"

    html = response.content.decode("utf-8")
    html = html.replace(
        '<link rel="icon" href="./favicon.ico"/>',  # keep original exact match!
        f'<link rel="icon" href="{application_icon}"/>',
    ).replace("<title>MaxKB</title>", f"<title>{application_name}</title>")
    response.content = html.encode("utf-8")

return response

Bottom line: The main thing to verify is item #1 — the added space in the favicon.ico search string likely breaks the replacement. Everything else is defensive hardening / style.

3 changes: 3 additions & 0 deletions ui/src/router/ROUTE_README.md
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,9 @@ Chat 导航守卫(`chat/index.ts`):

当前场景只在放行前切换,请求层直接读取 `auth.token`,导航未完成时不会带上目标场景的 token。

Chat Router 不根据 `route.meta.title` 更新浏览器标题:单应用对话页的标题和图标由后端
`ChatHeadersMiddleware` 替换为应用名称与应用图标,前端不得覆盖。

接口返回 401(`/auth/` 认证接口除外)时,请求层调用 `reauthenticate()`:将当前场景的 token 标记为失效(Store 中清除,本地存储保留),并以
`force: true` 重新导航到当前地址,由守卫重新匿名认证或跳转登录页。同一时间只处理一次。

Expand Down
4 changes: 0 additions & 4 deletions ui/src/router/chat/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,6 @@ chatRouter.beforeEach(async (to) => {
return getLoginRoute(scope, to)
})

chatRouter.afterEach((to) => {
document.title = to.meta.title ? `${to.meta.title} - MaxKB` : 'MaxKB'
})

let isReauthenticating = false

/**

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Analysis of the Code Diff

The diff shows the removal of an afterEach navigation guard in a Vue Router setup:

// REMOVED CODE:
chatRouter.afterEach((to) => {
  document.title = to.meta.title ? `${to.meta.title} - MaxKB` : 'MaxKB'
})

Potential Issues with Removing This

  1. Missing Page Titles: Without this guard, <document.title> will no longer update on route changes. Users will see a static or incorrect title after navigation, harming:

    • SEO (if server-side rendering or meta tags are relied upon)
    • User Experience (browser tabs won't reflect the current page)
  2. Broken Contract: If other parts of the codebase set route.meta.title, they may be expecting this guard to apply it silently by convention. Removing it breaks that implicit contract.

  3. No Replacement Shown: The diff only removes the block. If titles were handled elsewhere (e.g., in individual components via useRouteMeta() or a global plugin), that logic is not visible here. Ensure there is an equivalent mechanism.

Recommendations

  • ✅ If intentional: Add a comment explaining where title management now happens (e.g., moved to a composable or plugin).
  • ⚠️ If unintentional: Restore the guard or implement a modern alternative using Vue Router's meta handling at the component level:
    // Per-route approach (Vue Router 4+)
    const routes = [
      {
        path: '/dashboard',
        component: Dashboard,
        meta: { title: 'Dashboard' }
        // Use a beforeEach or watch in the component if needed
      }
    ]
  • 🔄 Consider centralizing metadata handling so that removing a single global hook doesn't silently break title updates across all routes.

Verdict

Irregularity Detected: A functional side-effect (document.title update) was removed without a visible replacement. This likely introduces a bug unless title management has been refactored elsewhere. Verify and document accordingly.

Expand Down
Loading