|
1 | | -# BDD Spec: Session Expired Circuit Breaker |
| 1 | +# BDD Spec: Pentest Findings Remediation (2026-05-24) |
2 | 2 |
|
3 | 3 | ## Goal |
4 | | -When a user's auth token expires, the frontend should detect the first 401 response, immediately suppress all further API calls, show ONE clear user-facing message, and transition to the login view — instead of the current behavior where multiple concurrent 401s each trigger independent cleanup, toasts, and redirects. |
| 4 | +修復黑箱滲透測試發現的 6 個安全問題，優先處理可導致資料刪除的 consent withdraw 驗證缺失。 |
5 | 5 |
|
6 | | -## Behavioral Unit |
7 | | -**Session Expired Circuit Breaker** — a global flag that short-circuits all API paths on first 401. |
8 | | - |
9 | | -## Problem Context |
10 | | -Two independent API call paths exist: |
11 | | -1. `apiCall()` in user-portal-init.js (internal fetch wrapper) |
12 | | -2. `APIClient.fetch()` → `ErrorPolicy.handle()` in api-client.js / error-policy.js (feature layer) |
13 | | - |
14 | | -Both handle 401 independently. Image loads via `<img src="/api/user/received-cards/.../thumbnail">` bypass both entirely. When token expires, concurrent requests create a 401 storm: multiple toasts, duplicate sessionStorage clears, and ErrorPolicy redirects to the same page causing reload loops. |
15 | | - |
16 | | -Production logs show: consent/check + multiple received-cards/image requests all hitting 401 within seconds. |
17 | | - |
18 | | -## Impacted Modules |
19 | | -- `workers/public/js/error-policy.js` — 401 handler |
20 | | -- `workers/public/js/api-client.js` — fetch wrapper |
21 | | -- `workers/public/js/user-portal-init.js` — internal apiCall() 401 handler |
| 6 | +## Behavioral Units |
| 7 | +1. **Consent Withdraw Confirmation Validation** (Medium-High) |
| 8 | +2. **Web Field URL Scheme Validation** (Low-Medium) |
| 9 | +3. **MCP redirect_uri Allowlist** (Medium) |
| 10 | +4. **Staging URL Leakage Cleanup** (Medium) |
| 11 | +5. **OAuth Init Rate Limiting** (Low) |
| 12 | +6. **NFC Tap Rate Limiting** (Low) |
22 | 13 |
|
23 | 14 | ## Scenarios |
24 | 15 |
|
25 | | -### Scenario 1: First 401 triggers circuit breaker |
| 16 | +### Scenario 1.1: Consent withdraw requires exact confirmation text |
26 | 17 | ```gherkin |
27 | | -Given a user is logged in on user-portal |
28 | | -When any API call returns HTTP 401 |
29 | | -Then a global session-expired flag is set (window.__sessionExpired = true) |
30 | | -And sessionStorage is cleared (auth_user, csrfToken) |
31 | | -And ONE toast is shown: "登入已過期，請重新登入" (info type, 3s) |
32 | | -And the view transitions to login (showView('login')) |
33 | | -And no page redirect/reload occurs |
| 18 | +Given a user is authenticated |
| 19 | +When POST /api/consent/withdraw with body {"confirmation": "wrong text"} |
| 20 | +Then response is 400 with error code "invalid_confirmation" |
| 21 | +And consent status remains unchanged |
34 | 22 | ``` |
35 | 23 |
|
36 | | -### Scenario 2: Subsequent 401s are suppressed |
| 24 | +### Scenario 1.2: Consent withdraw succeeds with correct confirmation |
37 | 25 | ```gherkin |
38 | | -Given the session-expired flag is already set |
39 | | -When another API call would be made (apiCall or APIClient.fetch) |
40 | | -Then the call short-circuits immediately without making a network request |
41 | | -And no additional toast or redirect is triggered |
42 | | -And the caller receives a structured 401 error (for proper error propagation) |
| 26 | +Given a user is authenticated with active consent |
| 27 | +When POST /api/consent/withdraw with body {"confirmation": "確認撤回"} |
| 28 | +Then response is 200 |
| 29 | +And consent_status is set to "withdrawn" |
| 30 | +And deletion_scheduled_at is set to now + 30 days |
43 | 31 | ``` |
44 | 32 |
|
45 | | -### Scenario 3: ErrorPolicy 401 respects circuit breaker |
| 33 | +### Scenario 1.3: Consent withdraw rejects empty/missing confirmation |
46 | 34 | ```gherkin |
47 | | -Given the session-expired flag is already set |
48 | | -When ErrorPolicy.handle() is called with status 401 |
49 | | -Then it returns { action: 'none' } without clearing sessionStorage again or redirecting |
| 35 | +Given a user is authenticated |
| 36 | +When POST /api/consent/withdraw with body {} |
| 37 | +Then response is 400 with error code "invalid_confirmation" |
50 | 38 | ``` |
51 | 39 |
|
52 | | -### Scenario 4: Login resets circuit breaker |
| 40 | +### Scenario 2.1: Web field rejects javascript: URL |
53 | 41 | ```gherkin |
54 | | -Given the session-expired flag is set |
55 | | -When the user successfully logs in again |
56 | | -Then the flag is reset to false |
57 | | -And API calls proceed normally |
| 42 | +Given a user is authenticated with a card |
| 43 | +When PUT /api/user/cards/{uuid} with body {"web": "javascript:alert(1)"} |
| 44 | +Then response is 400 with error mentioning "web" field |
58 | 45 | ``` |
59 | 46 |
|
60 | | -## Implementation Constraints |
61 | | - |
62 | | -1. **Shared flag**: Use `window.__sessionExpired` (boolean) — accessible from all JS modules without import. |
63 | | - |
64 | | -2. **api-client.js changes**: At the TOP of `APIClient.fetch()`, check `window.__sessionExpired`. If true, return immediately: |
65 | | - ```js |
66 | | - if (window.__sessionExpired) { |
67 | | - return { ok: false, status: 401, error: { code: 'SESSION_EXPIRED', message: '登入已過期', retryable: false } }; |
68 | | - } |
69 | | - ``` |
| 47 | +### Scenario 2.2: Web field accepts valid https URL |
| 48 | +```gherkin |
| 49 | +Given a user is authenticated with a card |
| 50 | +When PUT /api/user/cards/{uuid} with body {"web": "https://example.com"} |
| 51 | +Then response is 200 |
| 52 | +And card web field is updated |
| 53 | +``` |
70 | 54 |
|
71 | | -3. **error-policy.js changes**: In the 401 handler, check `window.__sessionExpired`: |
72 | | - - If already set → return `{ action: 'none' }` (no-op) |
73 | | - - If not set → set `window.__sessionExpired = true`, then return existing redirect action BUT change the action to 'none' for user-portal context (let apiCall handle the view transition instead of page redirect) |
| 55 | +### Scenario 3.1: MCP register rejects non-localhost redirect_uri |
| 56 | +```gherkin |
| 57 | +Given an unauthenticated client |
| 58 | +When POST /mcp/register with redirect_uris ["https://evil.com/callback"] |
| 59 | +Then response is 400 with error "invalid_redirect_uri" |
| 60 | +``` |
74 | 61 |
|
75 | | -4. **user-portal-init.js changes**: In `apiCall()` 401 handler: |
76 | | - - Check `window.__sessionExpired` first — if already set, just throw without re-doing cleanup |
77 | | - - If not set → set `window.__sessionExpired = true`, do cleanup, show toast ONCE, showView('login') |
78 | | - - At the TOP of `apiCall()`: if `window.__sessionExpired`, throw immediately without fetch |
| 62 | +### Scenario 3.2: MCP register accepts localhost redirect_uri |
| 63 | +```gherkin |
| 64 | +Given an unauthenticated client |
| 65 | +When POST /mcp/register with redirect_uris ["http://localhost:3000/callback"] |
| 66 | +Then response is 201 with client_id assigned |
| 67 | +``` |
79 | 68 |
|
80 | | -5. **Login success path**: After successful Google OAuth callback, set `window.__sessionExpired = false`. |
| 69 | +### Scenario 3.3: MCP register accepts 127.0.0.1 redirect_uri |
| 70 | +```gherkin |
| 71 | +Given an unauthenticated client |
| 72 | +When POST /mcp/register with redirect_uris ["http://127.0.0.1:8080/callback"] |
| 73 | +Then response is 201 with client_id assigned |
| 74 | +``` |
81 | 75 |
|
82 | | -6. **FeatureAPI.executeAction**: Handle `action: 'none'` case — return false (no retry). |
| 76 | +### Scenario 4: Staging URL removed from production |
| 77 | +```gherkin |
| 78 | +Given the production card-display page |
| 79 | +When rendered |
| 80 | +Then no dns-prefetch or link to staging worker URL exists |
| 81 | +And MCP OAuth redirect_uri uses production domain |
| 82 | +``` |
83 | 83 |
|
84 | | -7. **Do NOT change**: Image onerror handlers (already gracefully degrade to SVG icons), api-retry.js (separate concern), backend code. |
| 84 | +### Scenario 5: /api/oauth/init rate limited |
| 85 | +```gherkin |
| 86 | +Given an IP address |
| 87 | +When 20 requests to /api/oauth/init within 60 seconds |
| 88 | +Then requests beyond limit return 429 |
| 89 | +``` |
85 | 90 |
|
86 | | -## Validation Target |
87 | | -- After token expiry, only ONE toast appears regardless of how many concurrent API calls are in flight |
88 | | -- No page reload/redirect loop |
89 | | -- Backend logs show at most 1-2 401s (the ones already in flight), not a continuous stream |
90 | | -- Login view is shown cleanly |
91 | | -- After re-login, all functionality works normally |
| 91 | +### Scenario 6: /api/nfc/tap rate limited |
| 92 | +```gherkin |
| 93 | +Given an IP address |
| 94 | +When 30 requests to /api/nfc/tap within 60 seconds |
| 95 | +Then requests beyond limit return 429 |
| 96 | +``` |
92 | 97 |
|
93 | | -## Expected Outcome |
94 | | -Users see a clean "session expired, please re-login" message once, land on the login screen, and can log back in. No confusion about system being broken. |
| 98 | +## Implementation Notes |
| 99 | +- Finding #1: Add body parsing + confirmation === '確認撤回' check in handleConsentWithdraw |
| 100 | +- Finding #4: Add "web" to urlFields array in validateUserCardData |
| 101 | +- Finding #2: Restrict isValidRedirectUri to localhost/127.0.0.1 only |
| 102 | +- Finding #3: Remove staging dns-prefetch; fix MCP OAuth redirect_uri env config |
| 103 | +- Finding #5/#6: Add Durable Objects rate limiting to oauth init and nfc tap handlers |
0 commit comments