fix(invoice): 發票 API 補上租戶隔離與操作權限檢查 - #276
Open
timliudev wants to merge 4 commits into
Open
Conversation
The invoice endpoints read appId from the request body while AuthGuard only verifies the JWT signature, so a valid token from one tenant could issue, search or revoke invoices on another tenant - using that tenant's EzPay merchant credentials. All three handlers now take appId from the authenticated member, matching the convention used in the order, coupon, voucher, equity, event, program and storage controllers. searchInvoice gains the @Local('member') binding it was missing. A token without an appId is rejected with 403 rather than letting undefined reach the invoice gateway lookup. The appId property is dropped from the three body DTOs so it can no longer be supplied. Service signatures are unchanged - invoice.runner.ts calls them directly and still passes appId positionally. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
InvoiceController had no test coverage. These specs pin the behaviour the previous commit fixed: each handler passes the token's appId to the service, an appId left in the request body has no effect on which appId is used, and a token without an appId is rejected with a 403. InvoiceService is mocked, so no EzpayClient is constructed and no outbound HTTP call is made. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
AuthGuard only proves the caller holds a valid token for the app, so any signed-in member of a tenant could issue, revoke or search that tenant's invoices - issuing real MoF invoices under the tenant's EzPay merchant credentials. The endpoints predate manual invoicing in the admin UI and never carried an authorization check. The three handlers now require app-owner, or SALES_RECORDS_ADMIN - the permission the admin UI already gates manual invoicing behind (SalesPage, and MemberAdminPage's sale collection). Anyone else gets a 403. Not using @permissions + PermissionGuard, the usual convention here, because PermissionGuard has no app-owner fallback: an app-owner whose token happens not to carry SALES_RECORDS_ADMIN would be locked out of invoicing. Worth revisiting once that fallback exists. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The previous commit required SALES_RECORDS_ADMIN, which is narrower than what the admin UI actually offers and would have locked out staff who issue invoices today. Manual invoicing sits on the sale collection card, reachable two ways: the sales menu, shown for any of GROSS_SALES_ADMIN, GROSS_SALES_NORMAL, SALES_RECORDS_ADMIN, SALES_RECORDS_DETAILS or SALES_RECORDS_NORMAL (AdminMenu), and the member page's order tab, gated on SALES_RECORDS_ADMIN or CHECK_MEMBER_ORDER (MemberAdminPage). The endpoints now accept that union, so the API allows what the UI offers and nothing more. Deliberately not following the /sales route's effective gate: it declares allowedUserRole 'app-owner' but AdminRouter never reads that field and only checks BACKSTAGE_ENTER, so ~660 members across 45 apps can reach the manual invoice button by typing the URL even though the sales menu stays hidden for them. That is an unenforced guard, not an intended permission, and it is not mirrored here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
問題
/api/v2/invoice的issue/search/revoke三支 endpoint 只掛AuthGuard，而
AuthGuard僅呼叫authService.verify、只驗 JWT 簽章，全平台共用一把HASURA_JWT_SECRET。由此有兩個獨立的洞：跨租戶存取 — 三支 handler 的
appId取自 request body 而非已驗證的 token。任一站的有效 token 把 body 的
appId換成別站，即可對該站開票、作廢、查發票（回應含買受人姓名與 Email）。service 會依該
appId取出該站的 EzPay 商家憑證（
invoice.service.ts→checkInvoiceGatewayConfig→invoice.infra.ts:17），以其名義向財政部開立或作廢真實電子發票。
完全沒有授權檢查 — 只要是該站的登入會員（含一般學員）就能呼叫，不驗身分。
第 2 點是 8/19 日會回報、當場裁示要處理的項目；第 1 點是查證過程中一併發現。
根因
這三支 endpoint 早於後台的手動開發票功能。當時系統沒有手動開票的路徑，端點是為了
補這個缺口臨時開的，
appId直接沿用 body 參數、也沒有補授權判斷，之後手動開票 UI上線時沒有回頭收斂。
正式站實測（8/19，僅送未帶 token 的空 body 請求，未登入、未帶 appId、未執行任何
跨租戶操作）：三支皆回 401，不存在的路徑回 404 —— 代表反向代理將整個
/api/v2/*前綴轉入 lodestar-server、邊界無路徑白名單，且請求確實抵達
AuthGuard。即公開網路可達，無 IP／WAF 阻擋。
invoice.executor_id全表 263,482 筆皆為 NULL、invoice_log僅 2 筆（2025-09），手動開票路徑在正式站沒有留下可歸因紀錄，這也是這個洞一直沒被察覺的原因之一。
修法
租戶隔離
@Local('member') member: JwtMember取appId；search補上原本缺少的
@Local('member')綁定appId自三個 body DTO 移除，不再接受傳入appId回 403，不讓undefined流入 gateway 查詢invoice.runner.ts直接呼叫 service、仍以位置參數傳appId，該路徑不受影響操作權限
新增
assertInvoiceAdmin，要求app-owner，或持有下列任一權限：這組是對齊後台自己的 gate。手動開發票掛在 sale collection 卡片上，後台有兩條
路徑到達：
AdminMenu.tsx:57SALES_RECORDS_ADMIN || CHECK_MEMBER_ORDERMemberAdminPage.tsx:130取兩者聯集，讓 API 允許的範圍等於 UI 本來就提供的範圍。
未採用
@Permissions+PermissionGuard（本 repo 慣例）的原因：AccessControlService.isAuthorized只比對 permissions 陣列，沒有 app-ownerfallback，而 JWT 的 permissions 來自 member 自身權限列加上 app 預設權限
（
auth.service.ts:196、signJWT），app-owner 不會自動全拿 —— 直接套用會把部分app-owner 鎖在門外。待該 fallback 補上後可改回慣例寫法。
影響範圍
依正式站統計，改動後可呼叫這三支的人數：
app-owner（不看 permission）改動前是該站每一位登入會員。另已確認
app_default_permission沒有任何站把這些權限設為預設，不會有整站會員被自動授權的情況。
BACKSTAGE_ENTER但無任何銷售或訂單權限。他們側邊欄看不到「銷售管理」，但
AdminRouter只檢查BACKSTAGE_ENTER，直接輸入/sales仍可到達那張卡並按下手動開發票 ——
routesProps.sales雖宣告allowedUserRole: 'app-owner'，但
AdminRouter從未讀取該欄位（AdminRouter.tsx:686-688）。本 PR 選擇不比照該行為：那是前端未執行的守衛，不是設計上的權限。若確認有夥伴
靠這條路徑在日常開票，review 時請提出，加寬只是在陣列多一個值。
（前端
allowedUserRole未生效本身是另一個問題，本 PR 不處理。）測試
新增
invoice.controller.spec.ts—— 此 controller 原本無任何測試覆蓋。23 個案例，涵蓋：appId 一律取自 token、body 覆寫無效、缺 appId 回 403
E_NO_APP_ID、以及 app-owner 與六個權限的放行／拒絕矩陣（含只有BACKSTAGE_ENTER應被拒）。InvoiceService全程 mock，不會建立EzpayClient、不會送出任何對外請求。
jest src/invoice/invoice.controller.spec.ts23/23 通過tsc --noEmit無新增錯誤（test/runner、test/benchmark既有 4 個錯誤為develop 上既存問題，已 stash 本 PR 改動比對確認與本 PR 無關）
SlowBuffershim 才跑得起來（
jsonwebtoken→jws→jwa→buffer-equal-constant-time於 import 時讀buffer.SlowBuffer，Node 24 起已移除）。既有的coin.controller.spec同樣症狀，屬環境問題非本 PR 引入；shim 未進 repo，以 CI 結果為準。
SALES_RECORDS_ADMIN的帳號實際手動開票一次、以無權限帳號確認回 403。
🤖 Generated with Claude Code