Skip to content

feat: enable root admin to manage all user API keys - #6792

Closed
findlp163 wants to merge 1 commit into
QuantumNous:mainfrom
findlp163:feat/root-admin-manage-tokens
Closed

feat: enable root admin to manage all user API keys#6792
findlp163 wants to merge 1 commit into
QuantumNous:mainfrom
findlp163:feat/root-admin-manage-tokens

Conversation

@findlp163

@findlp163 findlp163 commented Aug 12, 2026

Copy link
Copy Markdown

📝 变更描述 / Description

后端:

  • 原有 /api/token/* 接口内部按角色分支:Root 走跨用户查询,支持 ?user_id=X 筛选目标用户;非 Root 逻辑不变
  • Token 结构新增 username 字段,Admin 查询时 LEFT JOIN users 表返回用户名
  • Admin 操作(创建/编辑/删除/批量删除他人令牌)通过 recordManageAuditFor 记录审计日志

前端(超管专属):

  • 令牌页新增「Username」筛选,复用 DataTableFacetedFilter,选项按用户名排序
  • 表格新增 Username 列,显示令牌所属用户
  • 创建抽屉新增目标用户选择器,默认自己

🚀 变更类型 / Type of change

  • 🐛 Bug 修复 (Bug fix) - 请关联对应 Issue,避免将设计取舍、理解偏差或预期不一致直接归类为 bug
  • ✨ 新功能 (New feature) - 重大特性建议先通过 Issue 沟通
  • ⚡ 性能优化 / 重构 (Refactor)
  • 📝 文档更新 (Documentation)

🔗 关联任务 / Related Issue

✅ 提交前检查项 / Checklist

  • 人工确认: 我已亲自整理并撰写此描述,没有直接粘贴未经处理的 AI 输出。
  • 非重复提交: 我已搜索现有的 IssuesPRs,确认不是重复提交。
  • Bug fix 说明: 若此 PR 标记为 Bug fix,我已提交或关联对应 Issue,且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。
  • 变更理解: 我已理解这些更改的工作原理及可能影响。
  • 范围聚焦: 本 PR 未包含任何与当前任务无关的代码改动。
  • 本地验证: 已在本地运行并通过测试或手动验证,维护者可以据此复核结果。
  • 安全合规: 代码中无敏感凭据,且符合项目代码规范。

📸 运行证明 / Proof of Work

超管角色 令牌管理页:
image

超管角色 创建令牌页:
image

普通角色 令牌管理页:
image

普通角色创建令牌页:
image

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Root users can manage tokens owned by other users. The backend adds administrator-scoped queries and audits. The frontend adds user filtering, username display, and target-user selection for key creation. Tests cover permissions and cross-user operations.

Changes

Administrator token management

Layer / File(s) Summary
Token model operations
model/token.go
The token model adds joined usernames, administrator search and listing, unrestricted batch deletion, Redis cleanup, and key lookup by IDs.
Controller access and audits
controller/token.go, controller/audit.go
Root users can manage other users’ tokens. Owner-aware limits, role checks, batch fallbacks, key retrieval, and management audits are included.
Administrator behavior validation
controller/token_test.go
Tests cover listing, filtering, masking, permissions, creation, updates, deletion, and non-root access.
Frontend key-management contract
web/src/features/keys/types.ts, web/src/features/keys/api.ts, web/src/features/keys/components/api-keys-provider.tsx
API requests support user_id. The provider exposes administrator status and selected-user state.
Frontend filtering and target selection
web/src/features/keys/components/api-keys-table.tsx, web/src/features/keys/components/api-keys-columns.tsx, web/src/features/keys/components/api-keys-mutate-drawer.tsx, web/src/components/data-table/toolbar/faceted-filter.tsx
Administrators can filter by username, view username data, and select a target user during key creation. The filter popover also has a maximum height.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Admin
  participant ApiKeysTable
  participant TokenController
  participant TokenModel
  participant Audit
  Admin->>ApiKeysTable: select user or manage key
  ApiKeysTable->>TokenController: send token request with user_id
  TokenController->>TokenModel: execute administrator token operation
  TokenModel-->>TokenController: return token data
  TokenController->>Audit: record cross-user action
  TokenController-->>ApiKeysTable: return response
Loading

Possibly related PRs

Poem

A rabbit hops through keys tonight,
Root paws grant the proper right.
Users filter, tokens glow,
Audits mark the paths they show.
“Cross-user keys!” the bunny sings.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The faceted-filter height and clear-button changes are unrelated to issue #3456, which covers administrator API key management. Move the faceted-filter UI changes to a separate pull request or link an issue that requires those changes.
Docstring Coverage ⚠️ Warning Docstring coverage is 44.44% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes satisfy issue #3456 by enabling root administrators to list, filter, create, update, delete, and inspect other users’ API keys.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: enabling root administrators to manage all users’ API keys.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 10

🧹 Nitpick comments (1)
web/src/features/keys/components/api-keys-table.tsx (1)

200-206: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Declare the user-query result type.

Both new queryFn callbacks omit an explicit return type. Declare Promise<User[]> to preserve the user-selector data contract.

  • web/src/features/keys/components/api-keys-table.tsx#L200-L206: Add : Promise<User[]> to the administrator user-list query callback.
  • web/src/features/keys/components/api-keys-mutate-drawer.tsx#L120-L126: Add : Promise<User[]> to the target-user query callback.

As per coding guidelines, “参数和返回值应显式标注类型”.

🤖 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 `@web/src/features/keys/components/api-keys-table.tsx` around lines 200 - 206,
Explicitly annotate both user-list queryFn callbacks with Promise<User[]>:
update web/src/features/keys/components/api-keys-table.tsx lines 200-206 and
web/src/features/keys/components/api-keys-mutate-drawer.tsx lines 120-126.
Preserve their existing query logic and User[] results.

Source: Coding guidelines

🤖 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 `@controller/token_test.go`:
- Around line 584-612: Update setupAdminTokenTestDB to explicitly initialize all
state required by AddToken, including the maximum token-count setting and
related cache/request context, before each test. Capture the original model.DB,
model.LOG_DB, common.RedisEnabled, database types, Gin mode, and relevant
settings, then restore them in t.Cleanup before closing the fixture database;
ensure cleanup cannot leave later tests referencing the closed replacement
database.
- Around line 594-602: Update the new administrator token tests in the relevant
test functions to import Testify’s require and assert packages, replacing all 29
t.Fatalf calls with require for setup or fatal conditions and assert for
non-fatal validation checks.

In `@controller/token.go`:
- Around line 543-557: Update controller/token.go lines 543-557 so the root-role
branch is checked before the owner-scoped BatchDeleteTokens fallback, validates
every requested ID, and calls BatchDeleteTokensAdmin directly. In
controller/token.go lines 581-584, branch on the root role before
GetTokenKeysByIds, use GetTokenKeysByIdsAdmin, and validate that all requested
IDs are returned.
- Around line 122-142: Handle the error returned by CountTokensAdmin in the
root-user branch before setting page totals; call common.ApiError with the
database error and return on failure, while preserving the existing successful
pagination flow.
- Around line 418-426: Move the recordManageAuditFor call for the
token.admin_delete event to after token.Delete() succeeds, ensuring failed
deletions do not create an audit entry. Preserve the existing audit fields and
only record the event after confirming the deletion returned no error.
- Around line 318-321: Retain the user returned by model.GetUserCache in the
token creation flow instead of discarding it, then pass targetUser.Username to
recordManageAuditFor so the token.admin_create audit receives the resolved
target username. Apply the same change to the corresponding flow around the
additional referenced block.
- Around line 416-424: In controller/token.go lines 416-424, always call
recordManageAuditFor after the cross-user deletion succeeds; use GetUserCache
only to optionally populate target_username. Apply the same change in
controller/token.go lines 514-522 for the cross-user update, ensuring both
audits are recorded even when the cache lookup fails.

In `@model/token.go`:
- Around line 221-269: Update the Order clause in SearchTokensAdmin to qualify
the id column with the tokens table, preserving descending order and the
existing query behavior.

In `@web/src/features/keys/components/api-keys-mutate-drawer.tsx`:
- Around line 232-235: Update the effect containing the targetUserId reset logic
to include isAdmin and authUser?.id in its dependency array, ensuring
targetUserId is refreshed when authentication state changes while the create
drawer remains open.

In `@web/src/features/keys/components/api-keys-table.tsx`:
- Around line 197-210: Replace the fixed first-page getUsers calls in
web/src/features/keys/components/api-keys-table.tsx:197-210 and
web/src/features/keys/components/api-keys-mutate-drawer.tsx:117-130 with the
complete paginated or server-backed user source, and use it to populate the
username filter options and target-user selector so users beyond the first 1000
remain available.

---

Nitpick comments:
In `@web/src/features/keys/components/api-keys-table.tsx`:
- Around line 200-206: Explicitly annotate both user-list queryFn callbacks with
Promise<User[]>: update web/src/features/keys/components/api-keys-table.tsx
lines 200-206 and web/src/features/keys/components/api-keys-mutate-drawer.tsx
lines 120-126. Preserve their existing query logic and User[] results.
🪄 Autofix

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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 12344b6d-70e1-4387-bc85-7660857f41e4

📥 Commits

Reviewing files that changed from the base of the PR and between ccd535e and d85ee18.

📒 Files selected for processing (11)
  • controller/audit.go
  • controller/token.go
  • controller/token_test.go
  • model/token.go
  • web/src/components/data-table/toolbar/faceted-filter.tsx
  • web/src/features/keys/api.ts
  • web/src/features/keys/components/api-keys-columns.tsx
  • web/src/features/keys/components/api-keys-mutate-drawer.tsx
  • web/src/features/keys/components/api-keys-provider.tsx
  • web/src/features/keys/components/api-keys-table.tsx
  • web/src/features/keys/types.ts

Comment thread controller/token_test.go
Comment on lines +584 to +612
// setupAdminTokenTestDB 初始化测试数据库,迁移 Token 和 User 表。
func setupAdminTokenTestDB(t *testing.T) *gorm.DB {
t.Helper()

gin.SetMode(gin.TestMode)
common.SetDatabaseTypes(common.DatabaseTypeSQLite, common.DatabaseTypeSQLite)
common.RedisEnabled = false

dsn := fmt.Sprintf("file:%s?mode=memory&cache=shared", strings.ReplaceAll(t.Name(), "/", "_"))
db, err := gorm.Open(sqlite.Open(dsn), &gorm.Config{})
if err != nil {
t.Fatalf("failed to open sqlite db: %v", err)
}
model.DB = db
model.LOG_DB = db

if err := db.AutoMigrate(&model.Token{}, &model.User{}); err != nil {
t.Fatalf("failed to migrate tables: %v", err)
}

t.Cleanup(func() {
sqlDB, err := db.DB()
if err == nil {
_ = sqlDB.Close()
}
})

return db
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Restore and explicitly initialize shared test state.

This fixture replaces model.DB, model.LOG_DB, common.RedisEnabled, database types, and Gin mode. It only closes the replacement database. Later tests can use the closed database or changed cache settings. It also does not explicitly initialize settings used by AddToken, such as the maximum token count.

Save and restore all process-global state in t.Cleanup. Set required settings in this fixture before each test. As per coding guidelines, “Initialize database, request context, user group, settings, and cache state explicitly in test fixtures.”

🤖 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 `@controller/token_test.go` around lines 584 - 612, Update
setupAdminTokenTestDB to explicitly initialize all state required by AddToken,
including the maximum token-count setting and related cache/request context,
before each test. Capture the original model.DB, model.LOG_DB,
common.RedisEnabled, database types, Gin mode, and relevant settings, then
restore them in t.Cleanup before closing the fixture database; ensure cleanup
cannot leave later tests referencing the closed replacement database.

Sources: Coding guidelines, Learnings

Comment thread controller/token_test.go
Comment on lines +594 to +602
if err != nil {
t.Fatalf("failed to open sqlite db: %v", err)
}
model.DB = db
model.LOG_DB = db

if err := db.AutoMigrate(&model.Token{}, &model.User{}); err != nil {
t.Fatalf("failed to migrate tables: %v", err)
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 2 'github\.com/stretchr/testify/(assert|require)' -g '*.go' .
rg -n -C 2 'testify' go.mod go.sum 2>/dev/null || true

Repository: QuantumNous/new-api

Length of output: 50375


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- file metadata ---'
wc -l controller/token_test.go
printf '%s\n' '--- AST outline ---'
ast-grep outline controller/token_test.go || true
printf '%s\n' '--- imports and test/fixture symbols ---'
rg -n '^(import|\s*"|func Test|func setup|func setup|model\.|common\.|setting\.|cache|gin\.|t\.(Fatalf|Errorf|Run|Cleanup|Parallel))' controller/token_test.go
printf '%s\n' '--- affected ranges ---'
sed -n '1,120p' controller/token_test.go
sed -n '540,990p' controller/token_test.go

Repository: QuantumNous/new-api

Length of output: 23758


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- file metadata ---'
wc -l controller/token_test.go
printf '%s\n' '--- AST outline ---'
ast-grep outline controller/token_test.go || true
printf '%s\n' '--- imports and test/fixture symbols ---'
rg -n '^(import|\s*"|func Test|func setup|model\.|common\.|setting\.|cache|gin\.|t\.(Fatalf|Errorf|Run|Cleanup|Parallel))' controller/token_test.go
printf '%s\n' '--- affected ranges ---'
sed -n '1,120p' controller/token_test.go
sed -n '540,990p' controller/token_test.go

Repository: QuantumNous/new-api

Length of output: 23758


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- assertion counts in the affected administrator suite ---'
python3 - <<'PY'
from pathlib import Path
import re

path = Path("controller/token_test.go")
lines = path.read_text().splitlines()
start, end = 584, 959
affected = lines[start:end]
fatal = [(start + i + 1, line.strip()) for i, line in enumerate(affected) if "t.Fatalf(" in line]
assert_calls = [(start + i + 1, line.strip()) for i, line in enumerate(affected)
                if re.search(r'\b(assert|require)\.', line)]
print(f"affected_lines={start+1}-{end}")
print(f"t.Fatalf_count={len(fatal)}")
for line_no, text in fatal:
    print(f"{line_no}: {text}")
print(f"testify_assert_or_require_calls={len(assert_calls)}")
for line_no, text in assert_calls:
    print(f"{line_no}: {text}")
print("testify_imports:")
for line in lines:
    if "github.com/stretchr/testify/" in line:
        print(line.strip())
PY

printf '%s\n' '--- AddToken and related setting references ---'
rg -n -C 5 'func AddToken|AddToken\(|GetTokenById|RoleRootUser|RoleAdminUser|UserStatusEnabled|operation_setting|system_setting|setting\.' controller model service common setting -g '*.go' | head -n 500

printf '%s\n' '--- global-state cleanup patterns in controller tests ---'
rg -n -C 3 'model\.(DB|LOG_DB)|common\.(RedisEnabled|SetDatabaseTypes)|gin\.SetMode|t\.Cleanup' controller -g '*_test.go' | head -n 500

Repository: QuantumNous/new-api

Length of output: 50375


Use require and assert in the new administrator token tests.

Replace the 29 t.Fatalf calls with require for setup and fatal checks, and assert for non-fatal checks. Add both Testify imports.

🤖 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 `@controller/token_test.go` around lines 594 - 602, Update the new
administrator token tests in the relevant test functions to import Testify’s
require and assert packages, replacing all 29 t.Fatalf calls with require for
setup or fatal conditions and assert for non-fatal validation checks.

Sources: Coding guidelines, Learnings

Comment thread controller/token.go
Comment on lines +122 to 142
// Root 可不传 user_id 看全部,也可传 ?user_id=X 筛选
if c.GetInt("role") == common.RoleRootUser {
filterUserId, _ := strconv.Atoi(c.Query("user_id"))
tokens, err := model.GetAllTokensAdmin(filterUserId, pageInfo.GetStartIdx(), pageInfo.GetPageSize())
if err != nil {
common.ApiError(c, err)
return
}
total, _ := model.CountTokensAdmin(filterUserId)
pageInfo.SetTotal(int(total))
pageInfo.SetItems(buildMaskedTokenResponses(tokens))
} else {
tokens, err := model.GetAllUserTokens(userId, pageInfo.GetStartIdx(), pageInfo.GetPageSize())
if err != nil {
common.ApiError(c, err)
return
}
total, _ := model.CountUserTokens(userId)
pageInfo.SetTotal(int(total))
pageInfo.SetItems(buildMaskedTokenResponses(tokens))
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Handle the administrator count error.

Line 130 discards an error from CountTokensAdmin. If the count query fails, this handler returns a successful page with total = 0. Return the database error instead.

🤖 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 `@controller/token.go` around lines 122 - 142, Handle the error returned by
CountTokensAdmin in the root-user branch before setting page totals; call
common.ApiError with the database error and return on failure, while preserving
the existing successful pagination flow.

Comment thread controller/token.go
Comment on lines +318 to +321
if _, err := model.GetUserCache(token.UserId); err != nil {
common.ApiErrorI18n(c, i18n.MsgInvalidParams)
return
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Use the resolved target username in the creation audit.

Line 318 validates the target user but discards it. cleanToken.Username is never populated by Insert, so token.admin_create records an empty ${target_username}. Retain the resolved user and pass targetUser.Username to recordManageAuditFor.

Also applies to: 391-395

🤖 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 `@controller/token.go` around lines 318 - 321, Retain the user returned by
model.GetUserCache in the token creation flow instead of discarding it, then
pass targetUser.Username to recordManageAuditFor so the token.admin_create audit
receives the resolved target username. Apply the same change to the
corresponding flow around the additional referenced block.

Comment thread controller/token.go
Comment on lines +416 to +424
targetUser, _ := model.GetUserCache(token.UserId)
if targetUser != nil {
recordManageAuditFor(c, token.UserId, "token.admin_delete", map[string]interface{}{
"target_user_id": token.UserId,
"target_username": targetUser.Username,
"token_id": token.Id,
"token_name": token.Name,
})
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Do not gate required administrator audits on the user cache.

If model.GetUserCache fails, both paths complete the mutation without an audit record. recordManageAuditFor already accepts the target user ID. Use the cache lookup only to enrich target_username, not to decide whether to record the action.

  • controller/token.go#L416-L424: Always record the cross-user deletion after the delete succeeds.
  • controller/token.go#L514-L522: Always record the cross-user update after the update succeeds.
📍 Affects 1 file
  • controller/token.go#L416-L424 (this comment)
  • controller/token.go#L514-L522
🤖 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 `@controller/token.go` around lines 416 - 424, In controller/token.go lines
416-424, always call recordManageAuditFor after the cross-user deletion
succeeds; use GetUserCache only to optionally populate target_username. Apply
the same change in controller/token.go lines 514-522 for the cross-user update,
ensuring both audits are recorded even when the cache lookup fails.

Comment thread controller/token.go
Comment on lines +418 to +426
recordManageAuditFor(c, token.UserId, "token.admin_delete", map[string]interface{}{
"target_user_id": token.UserId,
"target_username": targetUser.Username,
"token_id": token.Id,
"token_name": token.Name,
})
}
}
err = token.Delete()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Write the deletion audit after a successful delete.

This code records token.admin_delete before token.Delete(). If the delete fails, the audit log reports an action that did not occur. Defer the audit until after the delete result is confirmed.

🤖 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 `@controller/token.go` around lines 418 - 426, Move the recordManageAuditFor
call for the token.admin_delete event to after token.Delete() succeeds, ensuring
failed deletions do not create an audit entry. Preserve the existing audit
fields and only record the event after confirming the deletion returned no
error.

Comment thread controller/token.go
Comment on lines +543 to +557
// Root 可批量删除任意用户的令牌
if err != nil && c.GetInt("role") == common.RoleRootUser {
for _, id := range tokenBatch.Ids {
_, tErr := model.GetTokenById(id)
if tErr != nil {
common.ApiError(c, tErr)
return
}
}
count, err = model.BatchDeleteTokensAdmin(tokenBatch.Ids)
if err != nil {
common.ApiError(c, err)
return
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Branch on the root role before calling owner-scoped batch methods.

BatchDeleteTokens and GetTokenKeysByIds return a nil error when requested IDs belong to another user; they return an empty or partial result. The err != nil fallback therefore does not run for the normal cross-user case. Root users cannot reliably delete or retrieve other users’ token keys.

  • controller/token.go#L543-L557: For root users, validate all requested IDs and call the administrator batch delete path directly.
  • controller/token.go#L581-L584: For root users, call GetTokenKeysByIdsAdmin directly and validate that all requested IDs were returned.
📍 Affects 1 file
  • controller/token.go#L543-L557 (this comment)
  • controller/token.go#L581-L584
🤖 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 `@controller/token.go` around lines 543 - 557, Update controller/token.go lines
543-557 so the root-role branch is checked before the owner-scoped
BatchDeleteTokens fallback, validates every requested ID, and calls
BatchDeleteTokensAdmin directly. In controller/token.go lines 581-584, branch on
the root role before GetTokenKeysByIds, use GetTokenKeysByIdsAdmin, and validate
that all requested IDs are returned.

Comment thread model/token.go
Comment on lines +221 to +269
// SearchTokensAdmin 管理员搜索所有令牌,支持可选 userId 筛选。
// userIdFilter <= 0 时不按用户过滤。
func SearchTokensAdmin(userIdFilter int, keyword string, token string, offset int, limit int) (tokens []*Token, total int64, err error) {
if limit <= 0 || limit > searchHardLimit {
limit = searchHardLimit
}
if offset < 0 {
offset = 0
}

if token != "" {
token = strings.TrimPrefix(token, "sk-")
}

baseQuery := DB.Table("tokens").
Select("tokens.*, users.username").
Joins("LEFT JOIN users ON tokens.user_id = users.id")
if userIdFilter > 0 {
baseQuery = baseQuery.Where("tokens.user_id = ?", userIdFilter)
}

if keyword != "" {
keywordPattern, err := sanitizeLikePattern(keyword)
if err != nil {
return nil, 0, err
}
baseQuery = baseQuery.Where("tokens.name LIKE ? ESCAPE '!'", keywordPattern)
}
if token != "" {
tokenPattern, err := sanitizeLikePattern(token)
if err != nil {
return nil, 0, err
}
baseQuery = baseQuery.Where("tokens."+commonKeyCol+" LIKE ? ESCAPE '!'", tokenPattern)
}

err = baseQuery.Count(&total).Error
if err != nil {
common.SysError("failed to count search tokens: " + err.Error())
return nil, 0, errors.New("搜索令牌失败")
}

err = baseQuery.Order("id desc").Offset(offset).Limit(limit).Find(&tokens).Error
if err != nil {
common.SysError("failed to search tokens: " + err.Error())
return nil, 0, errors.New("搜索令牌失败")
}
return tokens, total, nil
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Qualify the ordered column.

Line 263 uses ORDER BY id after joining tokens and users. Both tables have an id column. SQLite, MySQL, and PostgreSQL can reject this query as ambiguous. Use tokens.id desc.

Proposed fix
-	err = baseQuery.Order("id desc").Offset(offset).Limit(limit).Find(&tokens).Error
+	err = baseQuery.Order("tokens.id desc").Offset(offset).Limit(limit).Find(&tokens).Error
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// SearchTokensAdmin 管理员搜索所有令牌,支持可选 userId 筛选。
// userIdFilter <= 0 时不按用户过滤。
func SearchTokensAdmin(userIdFilter int, keyword string, token string, offset int, limit int) (tokens []*Token, total int64, err error) {
if limit <= 0 || limit > searchHardLimit {
limit = searchHardLimit
}
if offset < 0 {
offset = 0
}
if token != "" {
token = strings.TrimPrefix(token, "sk-")
}
baseQuery := DB.Table("tokens").
Select("tokens.*, users.username").
Joins("LEFT JOIN users ON tokens.user_id = users.id")
if userIdFilter > 0 {
baseQuery = baseQuery.Where("tokens.user_id = ?", userIdFilter)
}
if keyword != "" {
keywordPattern, err := sanitizeLikePattern(keyword)
if err != nil {
return nil, 0, err
}
baseQuery = baseQuery.Where("tokens.name LIKE ? ESCAPE '!'", keywordPattern)
}
if token != "" {
tokenPattern, err := sanitizeLikePattern(token)
if err != nil {
return nil, 0, err
}
baseQuery = baseQuery.Where("tokens."+commonKeyCol+" LIKE ? ESCAPE '!'", tokenPattern)
}
err = baseQuery.Count(&total).Error
if err != nil {
common.SysError("failed to count search tokens: " + err.Error())
return nil, 0, errors.New("搜索令牌失败")
}
err = baseQuery.Order("id desc").Offset(offset).Limit(limit).Find(&tokens).Error
if err != nil {
common.SysError("failed to search tokens: " + err.Error())
return nil, 0, errors.New("搜索令牌失败")
}
return tokens, total, nil
}
// SearchTokensAdmin 管理员搜索所有令牌,支持可选 userId 筛选。
// userIdFilter <= 0 时不按用户过滤。
func SearchTokensAdmin(userIdFilter int, keyword string, token string, offset int, limit int) (tokens []*Token, total int64, err error) {
if limit <= 0 || limit > searchHardLimit {
limit = searchHardLimit
}
if offset < 0 {
offset = 0
}
if token != "" {
token = strings.TrimPrefix(token, "sk-")
}
baseQuery := DB.Table("tokens").
Select("tokens.*, users.username").
Joins("LEFT JOIN users ON tokens.user_id = users.id")
if userIdFilter > 0 {
baseQuery = baseQuery.Where("tokens.user_id = ?", userIdFilter)
}
if keyword != "" {
keywordPattern, err := sanitizeLikePattern(keyword)
if err != nil {
return nil, 0, err
}
baseQuery = baseQuery.Where("tokens.name LIKE ? ESCAPE '!'", keywordPattern)
}
if token != "" {
tokenPattern, err := sanitizeLikePattern(token)
if err != nil {
return nil, 0, err
}
baseQuery = baseQuery.Where("tokens."+commonKeyCol+" LIKE ? ESCAPE '!'", tokenPattern)
}
err = baseQuery.Count(&total).Error
if err != nil {
common.SysError("failed to count search tokens: " + err.Error())
return nil, 0, errors.New("搜索令牌失败")
}
err = baseQuery.Order("tokens.id desc").Offset(offset).Limit(limit).Find(&tokens).Error
if err != nil {
common.SysError("failed to search tokens: " + err.Error())
return nil, 0, errors.New("搜索令牌失败")
}
return tokens, total, nil
}
🤖 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 `@model/token.go` around lines 221 - 269, Update the Order clause in
SearchTokensAdmin to qualify the id column with the tokens table, preserving
descending order and the existing query behavior.

Source: Coding guidelines

Comment on lines +232 to +235
// Reset targetUserId to self when drawer opens
if (isAdmin && !isUpdate && authUser?.id) {
setTargetUserId(authUser.id)
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 5 'auth\.user|hydrate|persist|setAuth' web/src/stores/auth-store.ts
rg -n -C 8 'setTargetUserId|isAdmin|authUser' web/src/features/keys/components/api-keys-mutate-drawer.tsx

Repository: QuantumNous/new-api

Length of output: 157


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- auth store candidates ---'
fd -i 'auth.*store|store.*auth' web/src || true
printf '%s\n' '--- drawer structure ---'
wc -l web/src/features/keys/components/api-keys-mutate-drawer.tsx
ast-grep outline web/src/features/keys/components/api-keys-mutate-drawer.tsx --view compact || true
printf '%s\n' '--- relevant drawer code ---'
rg -n -C 12 'setTargetUserId|isAdmin|authUser|useEffect|user_id' web/src/features/keys/components/api-keys-mutate-drawer.tsx
printf '%s\n' '--- auth-related definitions and hydration references ---'
rg -n -C 8 'authUser|isAdmin|hydrate|persist|setAuth|auth\.user|useAuth' web/src --glob '*.{ts,tsx}' | head -n 500

Repository: QuantumNous/new-api

Length of output: 39678


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- auth store ---'
wc -l web/src/stores/auth-store.ts
sed -n '1,280p' web/src/stores/auth-store.ts

printf '%s\n' '--- drawer effect and submit context ---'
sed -n '220,355p' web/src/features/keys/components/api-keys-mutate-drawer.tsx

printf '%s\n' '--- auth bootstrap call sites ---'
rg -n -C 12 'bootstrapAuthentication|bootstrapState|setBundle\(' web/src --glob '*.{ts,tsx}' | head -n 500

printf '%s\n' '--- API key provider/store definitions ---'
rg -n -C 10 'function useApiKeys|const useApiKeys|createContext|selectedUserId|isAdmin' web/src/features/keys web/src --glob '*.{ts,tsx}' | head -n 500

Repository: QuantumNous/new-api

Length of output: 50375


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- root authentication gating ---'
sed -n '45,125p' web/src/routes/__root.tsx
sed -n '135,185p' web/src/routes/__root.tsx

printf '%s\n' '--- all auth user mutation call sites ---'
rg -n -C 6 'auth\.setUser|setUser\(' web/src --glob '*.{ts,tsx}' --glob '!**/*.gen.ts'

printf '%s\n' '--- authentication event handling ---'
sed -n '1,260p' web/src/lib/auth-session-sync.ts
rg -n -C 8 'subscribeAuthSessionEvents|applyAuthBundle|clearAuthentication|setBundle' web/src/routes/__root.tsx web/src/features/auth web/src/lib --glob '*.{ts,tsx}' | head -n 400

printf '%s\n' '--- exact drawer dependency block ---'
sed -n '260,290p' web/src/features/keys/components/api-keys-mutate-drawer.tsx

Repository: QuantumNous/new-api

Length of output: 39040


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- createApiKey payload and endpoint ---'
rg -n -C 10 'createApiKey|user_id' web/src/features/keys web/src --glob '*.{ts,tsx}' | head -n 500

printf '%s\n' '--- backend handling of API-key user_id ---'
rg -n -C 10 'user_id' . --glob '*.go' --glob '!web/**' | rg -n -C 5 'token|key|API|UserId|user_id' | head -n 500

printf '%s\n' '--- hook dependency lint configuration ---'
rg -n -C 8 'react-hooks/exhaustive-deps|eslint' web --glob '*.{js,cjs,mjs,json,yml,yaml,ts}' | head -n 300

Repository: QuantumNous/new-api

Length of output: 50375


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- token endpoint candidates ---'
rg -l -i 'api/token|createtoken|create.*token|token.*create' --glob '*.go' | head -n 100

printf '%s\n' '--- targeted token user_id handling ---'
for f in $(rg -l -i 'api/token|createtoken|create.*token|token.*create' --glob '*.go' | head -n 30); do
  rg -n -i -C 8 'user.?id|CreateToken|create.*token|api/token' "$f" || true
done

printf '%s\n' '--- lint configuration ---'
fd -H -i 'eslint|biome|package.json' web | head -n 100
rg -n -C 8 'react-hooks/exhaustive-deps|eslintConfig|lint' web/package.json web/.eslintrc* web/eslint.config.* 2>/dev/null || true

Repository: QuantumNous/new-api

Length of output: 199


Include authentication state in the effect dependencies. The effect reads isAdmin and authUser?.id but omits them. If either changes while the create drawer remains open, targetUserId can remain stale, including 0, and be submitted as user_id. Add both values to the dependency array.

🤖 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 `@web/src/features/keys/components/api-keys-mutate-drawer.tsx` around lines 232
- 235, Update the effect containing the targetUserId reset logic to include
isAdmin and authUser?.id in its dependency array, ensuring targetUserId is
refreshed when authentication state changes while the create drawer remains
open.

Comment on lines +197 to +210
// Fetch users list for admin user filter options
const { data: usersData } = useQuery({
queryKey: ['admin-users-list'],
queryFn: async () => {
const result = await getUsers({ p: 1, page_size: 1000, sort_by: 'username', sort_order: 'asc' })
if (result.success && result.data?.items) {
return result.data.items as User[]
}
return []
},
enabled: isAdmin,
staleTime: 60_000,
})
const users = usersData || []

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Load all selectable users.

getUsers is paginated. Each site requests only page 1. Administrators cannot filter or create keys for users beyond the fixed limit.

  • web/src/features/keys/components/api-keys-table.tsx#L197-L210: Use paginated or server-backed user search before building the username filter options.
  • web/src/features/keys/components/api-keys-mutate-drawer.tsx#L117-L130: Use the same complete user source for the target-user selector.
📍 Affects 2 files
  • web/src/features/keys/components/api-keys-table.tsx#L197-L210 (this comment)
  • web/src/features/keys/components/api-keys-mutate-drawer.tsx#L117-L130
🤖 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 `@web/src/features/keys/components/api-keys-table.tsx` around lines 197 - 210,
Replace the fixed first-page getUsers calls in
web/src/features/keys/components/api-keys-table.tsx:197-210 and
web/src/features/keys/components/api-keys-mutate-drawer.tsx:117-130 with the
complete paginated or server-backed user source, and use it to populate the
username filter options and target-user selector so users beyond the first 1000
remain available.

@findlp163
findlp163 force-pushed the feat/root-admin-manage-tokens branch from d85ee18 to 310117b Compare August 12, 2026 09:35
@findlp163 findlp163 changed the title feat: enable root admin to manage all user API keys; fix: constrain faceted filter popover height and pin clear button feat: enable root admin to manage all user API keys Aug 12, 2026
@findlp163 findlp163 closed this Aug 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

增加管理用户key的功能

1 participant