Skip to content

fix(subscription): record audit logs and lock user row on admin grant - #6325

Open
YiKongk wants to merge 1 commit into
QuantumNous:mainfrom
YiKongk:fix/subscription-admin-grant-audit-lock
Open

fix(subscription): record audit logs and lock user row on admin grant#6325
YiKongk wants to merge 1 commit into
QuantumNous:mainfrom
YiKongk:fix/subscription-admin-grant-audit-lock

Conversation

@YiKongk

@YiKongk YiKongk commented Jul 19, 2026

Copy link
Copy Markdown

⚠️ 提交说明 / PR Notice

Important

  • 请提供人工撰写的简洁摘要,避免直接粘贴未经整理的 AI 输出。

📝 变更描述 / Description

问题一:管理员分配订阅无任何日志记录。

管理员通过 /api/subscription/admin/bind/api/subscription/admin/users/:id/subscriptions 为用户直接开通订阅时,系统不写入任何日志:被操作用户的 manage 日志与操作者的审计日志均缺失。同文件中的订阅重置操作(AdminResetUserSubscriptionsByPlan)已实现两类日志的写入,此处属于遗漏。由于套餐配置 upgrade_group 时,开通订阅会同步变更用户分组(即权限等级变更),缺失日志意味着该类变更无法追溯操作者、时间与依据。

问题二:MaxPurchasePerUser 限购校验存在并发绕过。

CreateUserSubscriptionFromPlanTx 的限购校验为先 COUNTINSERT,中间无锁。两个并发请求可同时读到 count == max-1 并各自插入,最终超出限购上限。余额购买路径(PurchaseSubscriptionWithBalance)因事先锁定 user 行而不受影响;管理员分配路径无锁,可被绕过。

具体改动

  1. model/subscription.go:在限购 COUNT 之前通过 lockForUpdate 锁定 user 行。订单回调、余额购买、管理员分配三条开通路径均收敛于 CreateUserSubscriptionFromPlanTx,一处加锁即可全覆盖;限购判断逻辑本身未做任何修改。余额购买路径会对同一行重复加锁,行锁可重入,无副作用。
  2. controller/subscription.go:新增 recordSubscriptionGrantLogs,参照 AdminResetUserSubscriptionsByPlan 的模式写入两类日志。两条日志均采用项目现有的 op 描述符模式(action + 结构化 params,Content 仅存英文兜底文本):用户日志 action 为 subscription.granted,操作者审计为 subscription.admin_grant。前端按查看者语言在渲染期本地化,不将写库时的语言固化到数据库。
  3. controller/audit.go:注册 subscription.admin_grantsubscription.granted 两个英文兜底模板。
  4. web/default:在 AUDIT_TEMPLATES 注册上述两个 action 的渲染模板,并在 7 个语言文件中按 localeCompare 顺序插入对应文案。经典前端(web/classic)展示英文兜底文本,与其他审计日志行为一致。

行为变化说明

加锁需要先 SELECT 到 user 行,因此为不存在的用户 ID 分配订阅将直接返回错误,而非创建一条无归属的订阅记录。该行为已通过测试固定。

说明:本 PR 代码实现为 AI 辅助完成(AI-assisted),提交者已逐项审阅并在本地验证。

🚀 变更类型 / Type of change

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

🔗 关联任务 / Related Issue

  • 暂无对应 Issue;两处缺陷均为代码审查中发现,如维护者需要可补充提交 Issue。

✅ 提交前检查项 / Checklist

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

📸 运行证明 / Proof of Work

新增 model/subscription_admin_grant_test.go

=== RUN   TestCreateUserSubscriptionLocksUserRow
--- PASS: TestCreateUserSubscriptionLocksUserRow (0.00s)
=== RUN   TestCreateUserSubscriptionStillEnforcesPurchaseLimit
--- PASS: TestCreateUserSubscriptionStillEnforcesPurchaseLimit (0.00s)
=== RUN   TestCreateUserSubscriptionRejectsUnknownUser
--- PASS: TestCreateUserSubscriptionRejectsUnknownUser (0.00s)
ok  	github.com/QuantumNous/new-api/model

go build ./...go vetgo test ./model/ ./controller/ 均通过;web/defaultbun run typecheckbun run build 通过,7 个 locale JSON 校验合法。

行锁在测试中通过 dry-run session 断言生成的 SQL 包含 FOR UPDATE(沿用 model/locking_test.go 的做法)。测试环境为内存 SQLite,lockForUpdate 在 SQLite 下按设计跳过,故并发效果无法在单元测试中直接验证。

(部署验证截图:管理员分配订阅后,被操作用户日志与操作者审计日志均可查询,见下方评论区补充。)

Admin subscription grants left no trace: neither a manage log for the
target user nor an operator audit entry was written, even though the
grant can also upgrade the user's group. The reset handlers in the same
file already do both.

Both new logs store a language-neutral op descriptor (action + params)
with an English fallback content, like the other audit logs, so the
frontend localizes them per viewer instead of freezing one language
into the database.

The MaxPurchasePerUser count in CreateUserSubscriptionFromPlanTx also ran
without a row lock, so two concurrent grants could both read count-1 and
both insert. Locking the user row serializes them; the limit check itself
is unchanged. This covers the order, balance and admin paths at once.

Locking the row also rejects grants for a non-existent user up front
instead of creating an orphan subscription.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Subscription grants now lock target users during purchase-limit checks, record operation and management audit entries, and render localized grant messages with plan and user metadata.

Changes

Subscription Grant Flow

Layer / File(s) Summary
Lock subscription grants and validate limits
model/subscription.go, model/subscription_admin_grant_test.go
Target users are locked before purchase-limit checks; tests cover locking, repeated grants, and unknown users.
Record subscription grant audits
controller/subscription.go, controller/audit.go
Successful grant paths write subscription.granted and subscription.admin_grant audit entries with plan metadata.
Render localized grant messages
web/default/src/features/usage-logs/lib/format.ts, web/default/src/i18n/locales/*
Audit mappings and translations support recipient and target-user subscription grant messages.é

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Admin
  participant SubscriptionController
  participant SubscriptionModel
  participant AuditLog
  participant UsageLogs
  Admin->>SubscriptionController: grant subscription plan
  SubscriptionController->>SubscriptionModel: bind subscription with user-row lock
  SubscriptionModel-->>SubscriptionController: successful binding
  SubscriptionController->>AuditLog: record subscription.granted and subscription.admin_grant
  AuditLog->>UsageLogs: render localized audit content
Loading

Possibly related PRs

Suggested reviewers: calcium-ion

Poem

I’m a rabbit with grants in my hat,
Locking each user—imagine that!
Audit trails hop, translations bloom,
Plan titles dance across the room.
“Granted!” I thump, with joyful cheer.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding audit logs and row locking for admin subscription grants.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@YiKongk
YiKongk marked this pull request as ready for review July 19, 2026 23:20
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.

1 participant