feat: 增加管理员后台系统与 IP 批量管理功能#1
Merged
Merged
Conversation
📝 WalkthroughWalkthroughAdds an admin backend to the web app. JWT tokens gain an ChangesAdmin Backend Feature
Sequence Diagram(s)sequenceDiagram
participant Browser
participant LoginHandler
participant AuthService
participant AdminDependency
participant AdminRouter
participant DB
Browser->>LoginHandler: POST /web/login (credentials)
LoginHandler->>AuthService: LoginUser
AuthService-->>LoginHandler: access_token (with is_admin claim)
LoginHandler->>LoginHandler: decode JWT, read is_admin
alt is_admin == true
LoginHandler-->>Browser: redirect /web/select-role (set web_token cookie)
else
LoginHandler-->>Browser: redirect /web/dashboard (set web_token cookie)
end
Browser->>AdminRouter: GET /web/admin/dashboard
AdminRouter->>AdminDependency: get_current_admin_from_cookie(web_token)
AdminDependency->>DB: load User by sub
alt token missing/invalid or user inactive
AdminDependency-->>Browser: redirect /web/login
else user not admin
AdminDependency-->>Browser: redirect /web/dashboard
else admin ok
AdminDependency-->>AdminRouter: User
AdminRouter->>DB: aggregate count queries
DB-->>AdminRouter: stats
AdminRouter-->>Browser: render dashboard.html
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
app/web/routes/accounts.py (1)
33-46: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAvoid full in-memory proxy/account scans in the login hot path.
_auto_assign_proxycurrently loads all active proxies and all active accounts, then counts/sorts in Python. This will become expensive as account volume grows; move counting/min selection into SQL.Refactor sketch
+from sqlalchemy import and_, func, select + async def _auto_assign_proxy(db_session: AsyncSession) -> int: - proxies = (await db_session.scalars(select(ProxyInfo).where(ProxyInfo.is_active == True))).all() - if not proxies: - raise ValueError("系统暂无可用代理 IP，请联系管理员配置。") - - accounts = (await db_session.scalars(select(TelegramAccount).where(TelegramAccount.is_active == True))).all() - proxy_counts = {p.id: 0 for p in proxies} - for acc in accounts: - if acc.proxy_id in proxy_counts: - proxy_counts[acc.proxy_id] += 1 - - sorted_proxies = sorted(proxies, key=lambda p: proxy_counts[p.id]) - return sorted_proxies[0].id + stmt = ( + select(ProxyInfo.id) + .outerjoin( + TelegramAccount, + and_( + TelegramAccount.proxy_id == ProxyInfo.id, + TelegramAccount.is_active.is_(True), + ), + ) + .where(ProxyInfo.is_active.is_(True)) + .group_by(ProxyInfo.id) + .order_by(func.count(TelegramAccount.id), ProxyInfo.id) + .limit(1) + ) + pid = await db_session.scalar(stmt) + if pid is None: + raise ValueError("系统暂无可用代理 IP，请联系管理员配置。") + return pid🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/web/routes/accounts.py` around lines 33 - 46, The `_auto_assign_proxy` function is inefficient because it loads all active proxies and all active accounts into memory, then counts and sorts them in Python. Instead, use SQL aggregation to count the number of accounts assigned to each proxy, order by the count in ascending order, and select only the single proxy with the minimum assignment count. This approach should use a LEFT JOIN or subquery pattern in your select statement to count accounts per proxy ID, then limit the result to retrieve only the single proxy with the lowest count, eliminating the need for in-memory iteration and sorting.app/web/routes/admin.py (3)
275-276: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winSilent exception swallowing loses debugging context.
The bare
except Exception: passdiscards all error information, making it difficult to diagnose why valid-looking proxy lines fail to parse.♻️ Suggested fix to log parsing failures
if host and port: return { "proxy_host": host, "proxy_port": int(port), "username": username, "password": password, "proxy_type": proxy_type } - except Exception: - pass + except Exception as e: + logger.debug("Failed to parse proxy URL '%s': %s", line, e)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/web/routes/admin.py` around lines 275 - 276, The bare except Exception: pass block at lines 275-276 silently discards all exception information without logging, making it impossible to debug parsing failures. Replace the pass statement with proper error logging that captures and logs the actual exception details (including the exception message and type) using the appropriate logger, so that when proxy line parsing fails, the diagnostic information is preserved for debugging.Source: Linters/SAST tools
397-403: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winN+1 query pattern in batch delete.
Each proxy is fetched individually in a loop. For large batches, this creates many round-trips to the database.
♻️ Suggested fix using bulk UPDATE
if proxy_ids: - # 执行软删除：将 is_active 设为 False - for pid in proxy_ids: - proxy = await db_session.get(ProxyInfo, pid) - if proxy: - proxy.is_active = False + from sqlalchemy import update + await db_session.execute( + update(ProxyInfo) + .where(ProxyInfo.id.in_(proxy_ids)) + .values(is_active=False) + ) await db_session.commit()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/web/routes/admin.py` around lines 397 - 403, The code fetches each proxy individually in a loop using db_session.get(ProxyInfo, pid), which creates an N+1 query pattern causing many database round-trips for large batches. Replace the loop that iterates through proxy_ids and fetches each ProxyInfo individually with a single bulk UPDATE statement that sets is_active to False for all proxies whose IDs are in the proxy_ids list, then commit the session once. This will reduce multiple queries to a single efficient database operation.
61-65: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueConsider using UTC for consistent date boundaries.
datetime.now()uses the server's local timezone. If the server timezone differs from the expected user timezone or changes (e.g., DST, deployment to different region), the "today's messages" count may be inconsistent.♻️ Suggested fix using UTC
- from datetime import datetime, time - today_start = datetime.combine(datetime.now().date(), time.min) + from datetime import datetime, time, timezone + today_start = datetime.combine(datetime.now(timezone.utc).date(), time.min)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/web/routes/admin.py` around lines 61 - 65, Replace the `datetime.now()` call in the today_start calculation with `datetime.utcnow()` to ensure timezone consistency. The current code uses the server's local timezone which can cause inconsistent results if the server timezone differs or changes. Update the line where `today_start` is created to use UTC time instead of local time, ensuring the date boundary comparison with TelegramMessage.created_at remains consistent across all deployments and timezones.templates/admin/users.html (1)
67-69: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAvoid rendering full API keys by default in the user table.
Exposing full keys for every row increases credential leakage risk in normal admin operations. Mask by default and reveal/copy only on explicit action.
Suggested fix
- <code class="text-xs bg-slate-100 px-2 py-1 rounded text-slate-600 font-mono select-all">{{ u.api_key }}</code> + <code class="text-xs bg-slate-100 px-2 py-1 rounded text-slate-600 font-mono"> + {{ (u.api_key[:6] ~ '••••••' ~ u.api_key[-4:]) if u.api_key else '-' }} + </code>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@templates/admin/users.html` around lines 67 - 69, The code element displaying the full API key in the api-key-{{ u.id }} div exposes the complete credential by default, creating a security risk. Mask the API key by default (replace the visible text with dots or asterisks) and only reveal the actual key value on explicit user action such as a click or button interaction. Consider adding a separate button or event handler alongside the reset button that toggles visibility or copies the key to clipboard without keeping it permanently visible in the table.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/web/dependencies.py`:
- Around line 32-35: The function get_current_admin_from_cookie uses three
undefined names (AsyncSession, get_db_session, and User) which will cause
runtime NameError exceptions. Add the missing import statements at the top of
the file: import AsyncSession from sqlalchemy.ext.asyncio, import get_db_session
from the appropriate database module in your project, and import User from
app.models. These imports must be added before the function definition to make
the referenced names available in the file's namespace.
In `@app/web/routes/accounts.py`:
- Around line 70-73: The code at the proxy_id assignment section (around line
70) and the similar location at line 195 currently accepts proxy_id from client
form data, which bypasses server-side auto-assignment. Remove the logic that
attempts to parse and validate proxy_id from the form input (the int()
conversion of proxy_id parameter), and instead always call
_auto_assign_proxy(db_session) to enforce server-side proxy assignment. Apply
this same change at both locations mentioned (the initial assignment area and
the second location at line 195) to ensure the proxy is always assigned by the
server regardless of what the client submits.
In `@app/web/routes/admin.py`:
- Around line 278-299: The colon-based string splitting in the proxy parsing
logic will incorrectly parse IPv6 addresses because they contain multiple
colons. Before attempting the simple colon split on the line variable, first
check if the string contains IPv6 address format (either with brackets like
[::1]:port or without). For IPv6 addresses with brackets, extract the IPv6
address from within the brackets and the port/credentials after the closing
bracket separately. For IPv6 addresses without brackets, use a different parsing
strategy that handles the multiple colons in the IPv6 address. Only apply the
current colon-splitting logic for non-IPv6 addresses (IPv4 and hostnames).
In `@templates/admin/admin_base.html`:
- Around line 62-70: The aria-expanded attribute on the button with
onclick="toggleAdminMobileMenu()" is hardcoded to "false" and never updates when
the menu toggles, breaking assistive technology accessibility. Modify the
toggleAdminMobileMenu() function to update the aria-expanded attribute on the
button element to "true" when opening the menu and "false" when closing it,
keeping the state synchronized with the actual visibility state of the mobile
menu elements. Apply the same fix to the other mobile menu button mentioned in
lines 131-143.
In `@templates/admin/proxies.html`:
- Around line 49-51: The password input field on line 49-51 has type="text"
which displays passwords in clear text as users type them, posing a security
risk. Change the input type attribute from "text" to "password" to mask the
password input. Additionally, the comment indicates this issue also applies to
line 132, which likely involves rendering passwords in a list or table - locate
that area and ensure proxy credentials are not displayed in plain text there as
well, either by masking the values or hiding them from view.
- Around line 20-21: All POST forms in the proxies template (including those
with actions /web/admin/proxies/add, /web/admin/proxies/import, and
/web/admin/proxies/batch-delete) and HTMX POST requests (toggle-active and
delete) are missing CSRF token protection. Add CSRF token fields to each form by
including a hidden input with the token value, ensure HTMX requests include the
token in request headers or data attributes, and update all corresponding route
handlers to validate the CSRF token before processing the request.
Alternatively, configure SameSite=Strict on session cookies if using a framework
with built-in CSRF protection.
---
Nitpick comments:
In `@app/web/routes/accounts.py`:
- Around line 33-46: The `_auto_assign_proxy` function is inefficient because it
loads all active proxies and all active accounts into memory, then counts and
sorts them in Python. Instead, use SQL aggregation to count the number of
accounts assigned to each proxy, order by the count in ascending order, and
select only the single proxy with the minimum assignment count. This approach
should use a LEFT JOIN or subquery pattern in your select statement to count
accounts per proxy ID, then limit the result to retrieve only the single proxy
with the lowest count, eliminating the need for in-memory iteration and sorting.
In `@app/web/routes/admin.py`:
- Around line 275-276: The bare except Exception: pass block at lines 275-276
silently discards all exception information without logging, making it
impossible to debug parsing failures. Replace the pass statement with proper
error logging that captures and logs the actual exception details (including the
exception message and type) using the appropriate logger, so that when proxy
line parsing fails, the diagnostic information is preserved for debugging.
- Around line 397-403: The code fetches each proxy individually in a loop using
db_session.get(ProxyInfo, pid), which creates an N+1 query pattern causing many
database round-trips for large batches. Replace the loop that iterates through
proxy_ids and fetches each ProxyInfo individually with a single bulk UPDATE
statement that sets is_active to False for all proxies whose IDs are in the
proxy_ids list, then commit the session once. This will reduce multiple queries
to a single efficient database operation.
- Around line 61-65: Replace the `datetime.now()` call in the today_start
calculation with `datetime.utcnow()` to ensure timezone consistency. The current
code uses the server's local timezone which can cause inconsistent results if
the server timezone differs or changes. Update the line where `today_start` is
created to use UTC time instead of local time, ensuring the date boundary
comparison with TelegramMessage.created_at remains consistent across all
deployments and timezones.
In `@templates/admin/users.html`:
- Around line 67-69: The code element displaying the full API key in the
api-key-{{ u.id }} div exposes the complete credential by default, creating a
security risk. Mask the API key by default (replace the visible text with dots
or asterisks) and only reveal the actual key value on explicit user action such
as a click or button interaction. Consider adding a separate button or event
handler alongside the reset button that toggles visibility or copies the key to
clipboard without keeping it permanently visible in the table.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9c5ce1ff-c4c7-4336-9ff0-d01ffb0830ad
📒 Files selected for processing (13)
app/service/auth_service.pyapp/web/__init__.pyapp/web/dependencies.pyapp/web/routes/accounts.pyapp/web/routes/admin.pyapp/web/routes/auth.pytemplates/accounts/login_flow.htmltemplates/admin/admin_base.htmltemplates/admin/dashboard.htmltemplates/admin/proxies.htmltemplates/admin/select_role.htmltemplates/admin/users.htmltemplates/base.html
Comment on lines
+20
to
+21
| <form action="/web/admin/proxies/add" method="POST" class="space-y-4"> | ||
| <div> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
echo "1) Locate CSRF token generation/validation paths"
rg -n -C2 -i '\bcsrf\b|xsrf|x-csrf|csrf_token|csrftoken' --type=py --type=html
echo "2) Check cookie security attributes in auth/session handling"
rg -n -C2 -i 'set_cookie\(|samesite|httponly|secure' --type=py
echo "3) Inspect admin POST routes and their guards"
rg -n -C2 '`@router`\.post\("/admin' app/web/routes/admin.pyRepository: dunea/telegram-auto-message-server
Length of output: 127
🏁 Script executed:
#!/bin/bash
# Explore repository structure to understand layout
echo "=== Repository structure ==="
git ls-files | head -30
echo ""
# Check if the template file exists
echo "=== Looking for proxies template ==="
find . -name "proxies.html" -type f 2>/dev/null || echo "File not found"
echo ""
# Find all HTML templates to understand structure
echo "=== All HTML template files ==="
find . -name "*.html" -type f 2>/dev/null | head -20Repository: dunea/telegram-auto-message-server
Length of output: 1533
🏁 Script executed:
#!/bin/bash
# Look for Python app files
echo "=== Looking for app/main structure ==="
find . -path "*/app/*.py" -o -path "*/routes/*.py" -o -path "*/web/*.py" | head -20
echo ""
# Check admin.py if it exists
if [ -f "app/web/routes/admin.py" ]; then
echo "=== Found admin.py, checking for POST routes ==="
wc -l app/web/routes/admin.py
else
find . -name "admin.py" -type f 2>/dev/null
fiRepository: dunea/telegram-auto-message-server
Length of output: 739
🏁 Script executed:
#!/bin/bash
# Search for CSRF/XSRF patterns more broadly
echo "=== Searching for CSRF-related code (broader search) ==="
rg -i 'csrf|xsrf|x-csrf|token.*form|form.*token' -t py -t html -t js 2>/dev/null | head -50 || echo "No CSRF patterns found"Repository: dunea/telegram-auto-message-server
Length of output: 50391
🏁 Script executed:
#!/bin/bash
# Read the proxies template to check for CSRF tokens
echo "=== Reading templates/admin/proxies.html ==="
cat templates/admin/proxies.htmlRepository: dunea/telegram-auto-message-server
Length of output: 13156
🏁 Script executed:
#!/bin/bash
# Read admin.py routes to check for CSRF protection on POST endpoints
echo "=== Checking admin.py file structure ==="
head -100 app/web/routes/admin.py
echo ""
echo "=== Looking for POST route decorators and CSRF patterns ==="
grep -n "post\|csrf\|CSRF\|@" app/web/routes/admin.py | head -40Repository: dunea/telegram-auto-message-server
Length of output: 4914
🏁 Script executed:
#!/bin/bash
# Look for Flask/web framework setup and CSRF middleware
echo "=== Looking for CSRF middleware/extension setup ==="
find . -name "*.py" -path "*/app/*" ! -path "*/alembic/*" | xargs grep -l "csrf\|CSRF" 2>/dev/null | head -10
echo ""
echo "=== Check main app initialization ==="
cat app/__init__.pyRepository: dunea/telegram-auto-message-server
Length of output: 439
Add CSRF token validation to all admin POST endpoints.
The template contains multiple POST forms (/web/admin/proxies/add, /web/admin/proxies/import, /web/admin/proxies/batch-delete) and HTMX POST requests (toggle-active, delete) without CSRF tokens, while the backend uses cookie-based authentication with no CSRF protection mechanism. Implement CSRF token generation in forms and validation in route handlers, or enable SameSite=Strict on session cookies as a mitigation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@templates/admin/proxies.html` around lines 20 - 21, All POST forms in the
proxies template (including those with actions /web/admin/proxies/add,
/web/admin/proxies/import, and /web/admin/proxies/batch-delete) and HTMX POST
requests (toggle-active and delete) are missing CSRF token protection. Add CSRF
token fields to each form by including a hidden input with the token value,
ensure HTMX requests include the token in request headers or data attributes,
and update all corresponding route handlers to validate the CSRF token before
processing the request. Alternatively, configure SameSite=Strict on session
cookies if using a framework with built-in CSRF protection.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/web/routes/accounts.py (2)
63-80: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueConsider removing unused
proxy_idparameter.The
proxy_id: str = Form("")parameter on line 65 is now dead code since the function unconditionally calls_auto_assign_proxy(). Removing it clarifies the API contract and eliminates the misleading suggestion that clients can influence proxy selection.♻️ Suggested cleanup
async def request_phone_code( phone_number: str = Form(...), - proxy_id: str = Form(""), telegram_service: TelegramService = Depends(get_telegram_service), user_id: int = Depends(get_current_user_from_cookie), db_session: AsyncSession = Depends(get_db_session), ):🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/web/routes/accounts.py` around lines 63 - 80, The `proxy_id` parameter in the `request_phone_code` function signature is unused dead code since the function unconditionally calls `_auto_assign_proxy()` without referencing it. Remove the `proxy_id: str = Form("")` parameter from the function definition to clarify the API contract and eliminate the misleading indication that clients can influence proxy selection.
185-203: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueSame cleanup opportunity: remove unused
proxy_idparameter.Same as
request_phone_codeabove—theproxy_idparameter on line 188 is unused and can be removed.♻️ Suggested cleanup
async def login_with_session( phone_number: str = Form(...), session_string: str = Form(...), - proxy_id: str = Form(""), telegram_service: TelegramService = Depends(get_telegram_service), user_id: int = Depends(get_current_user_from_cookie), db_session: AsyncSession = Depends(get_db_session), ):🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/web/routes/accounts.py` around lines 185 - 203, The `proxy_id` parameter in the `login_with_session` function is declared but never used in the function body. Remove the unused `proxy_id: str = Form("")` parameter from the function signature to clean up the code and improve maintainability.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@app/web/routes/accounts.py`:
- Around line 63-80: The `proxy_id` parameter in the `request_phone_code`
function signature is unused dead code since the function unconditionally calls
`_auto_assign_proxy()` without referencing it. Remove the `proxy_id: str =
Form("")` parameter from the function definition to clarify the API contract and
eliminate the misleading indication that clients can influence proxy selection.
- Around line 185-203: The `proxy_id` parameter in the `login_with_session`
function is declared but never used in the function body. Remove the unused
`proxy_id: str = Form("")` parameter from the function signature to clean up the
code and improve maintainability.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e822d37b-b0c7-48da-b977-cc789736064b
📒 Files selected for processing (8)
app/web/dependencies.pyapp/web/routes/accounts.pyapp/web/routes/admin.pyapp/web/routes/auth.pytemplates/admin/admin_base.htmltemplates/admin/proxies.htmltemplates/admin/users.htmltemplates/base.html
🚧 Files skipped from review as they are similar to previous changes (6)
- app/web/dependencies.py
- templates/base.html
- templates/admin/admin_base.html
- app/web/routes/auth.py
- templates/admin/proxies.html
- templates/admin/users.html
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
已完成管理员后台、角色选择页面、用户管理以及代理 IP 批量导入/软删除功能开发与联调。
Summary by CodeRabbit
Release Notes