|
|
|
|
@@ -0,0 +1,327 @@
|
|
|
|
|
---
|
|
|
|
|
phase: quick-261022-vfz
|
|
|
|
|
plan: 01
|
|
|
|
|
quick_id: 261022-vfz
|
|
|
|
|
slug: user-plugin-login-throttle-via-surf-clie
|
|
|
|
|
type: execute
|
|
|
|
|
mode: quick
|
|
|
|
|
wave: 1
|
|
|
|
|
depends_on: []
|
|
|
|
|
autonomous: true
|
|
|
|
|
requirements:
|
|
|
|
|
- vfz-1 # item 9: POST /login keys its failed-login throttle on surf.ClientIP with http.trusted_proxies (as /register does); exported RemoteIP stays the RemoteAddr host helper with a corrected doc comment
|
|
|
|
|
- vfz-2 # item 10: /reset-password and the admin Users password set stamp tokens_valid_after truncated to whole seconds (as RevokeUserAccess does); /change-password and bouncer's comparison unchanged
|
|
|
|
|
- vfz-3 # escalation note items 9 and 10 (tracker, verdicts, reply) and the todo's Outcome filled; left uncommitted for the orchestrator
|
|
|
|
|
files_modified:
|
|
|
|
|
# sm-user-plugin (module git.golem15.com/golem15/sm-user-plugin), worked in the sm-demo-app submodule checkout. Commit there only.
|
|
|
|
|
- ../sm-demo-app/plugins/golem15/user/controllers/api_controller.go
|
|
|
|
|
- ../sm-demo-app/plugins/golem15/user/controllers/users_admin_controller.go
|
|
|
|
|
- ../sm-demo-app/plugins/golem15/user/login_client_ip_test.go
|
|
|
|
|
- ../sm-demo-app/plugins/golem15/user/same_second_cutoff_test.go
|
|
|
|
|
- ../sm-demo-app/plugins/golem15/user/README.md
|
|
|
|
|
# summercms.go planning docs, edited by Task 3 and NOT committed (the orchestrator commits docs)
|
|
|
|
|
- .planning/notes/2026-10-22-external-user-plugin-escalations.md
|
|
|
|
|
- .planning/todos/pending/2026-10-22-user-login-lockout-and-same-second-cutoff.md
|
|
|
|
|
|
|
|
|
|
estimate:
|
|
|
|
|
tokens: 85000
|
|
|
|
|
raw_tokens: 85000
|
|
|
|
|
tasks: 3
|
|
|
|
|
confidence: low # estimate-calibration: factor 1, sample_count 0
|
|
|
|
|
|
|
|
|
|
must_haves:
|
|
|
|
|
truths:
|
|
|
|
|
- "With http.trusted_proxies listing the proxy, five wrong passwords sent through that proxy with X-Forwarded-For 203.0.113.10 leave exactly one user_throttle row for the account, keyed 203.0.113.10, with 5 attempts and suspended; the correct password from 203.0.113.10 then gets the opaque 401, while the correct password from 203.0.113.20 through the same proxy gets 200 and a token."
|
|
|
|
|
- "Without http.trusted_proxies (the default), X-Forwarded-For is ignored for the /login throttle: failures from one connection address with a different X-Forwarded-For each time all count on one row keyed on the connection address, and a further forged X-Forwarded-For does not escape that suspension."
|
|
|
|
|
- "controllers.RemoteIP keeps its signature and behaviour (the host of r.RemoteAddr); its doc comment no longer says /login uses it."
|
|
|
|
|
- "After POST /reset-password and after an administrator sets a password on the Users form, users.tokens_valid_after is a whole second. A session token whose iat is that second is accepted on a jwt.auth route, at /fetch and at /refresh; a token issued one second earlier is refused at all three; and a login right after the reset yields a token that works. email_verified_at is still written by the reset."
|
|
|
|
|
- "/change-password still writes iat minus one second, and summercms.go modules/bouncer is unmodified."
|
|
|
|
|
- "go vet and the full Docker-backed go test of sm-user-plugin pass at each of two local fix commits on master (throttle first, cutoff second), neither pushed nor tagged; go.mod/go.sum unchanged; the sm-demo-app pointer is not bumped."
|
|
|
|
|
- "Escalation note rows 9 and 10 read done (unreleased) with their commit SHA and v0.5.0, their verdicts and reply paragraphs are filled with no TODO left for them, and the todo carries an Outcome section and stays in pending/, all uncommitted."
|
|
|
|
|
artifacts:
|
|
|
|
|
- path: ../sm-demo-app/plugins/golem15/user/controllers/api_controller.go
|
|
|
|
|
provides: "Login throttle IP from surf.ClientIP with the trusted proxies; reset-password cutoff truncated to the second; RemoteIP doc comment corrected"
|
|
|
|
|
contains: "now.Truncate(time.Second)"
|
|
|
|
|
- path: ../sm-demo-app/plugins/golem15/user/controllers/users_admin_controller.go
|
|
|
|
|
provides: "FormBeforeUpdate stamps TokensValidAfter truncated to the second when the password changes"
|
|
|
|
|
contains: "now := time.Now().Truncate(time.Second)"
|
|
|
|
|
- path: ../sm-demo-app/plugins/golem15/user/login_client_ip_test.go
|
|
|
|
|
provides: "TestLoginThrottleClientIP (Docker, skips under -short) and the adminEnv.loginFrom helper"
|
|
|
|
|
contains: "func TestLoginThrottleClientIP"
|
|
|
|
|
- path: ../sm-demo-app/plugins/golem15/user/same_second_cutoff_test.go
|
|
|
|
|
provides: "TestPasswordCutoffSameSecond (Docker, skips under -short): reset-password and admin password set subtests"
|
|
|
|
|
contains: "func TestPasswordCutoffSameSecond"
|
|
|
|
|
- path: ../sm-demo-app/plugins/golem15/user/README.md
|
|
|
|
|
provides: "Login throttling bullet names the client-address rule and http.trusted_proxies; RemoteIP described as the RemoteAddr host with no proxy handling; Testing names TestLoginThrottleClientIP"
|
|
|
|
|
contains: "http.trusted_proxies"
|
|
|
|
|
- path: .planning/notes/2026-10-22-external-user-plugin-escalations.md
|
|
|
|
|
provides: "rows 9 and 10 done (unreleased) with SHAs, verdicts and reply paragraphs filled"
|
|
|
|
|
contains: "done (unreleased)"
|
|
|
|
|
- path: .planning/todos/pending/2026-10-22-user-login-lockout-and-same-second-cutoff.md
|
|
|
|
|
provides: "## Outcome (2026-10-22) section before ## Invoke"
|
|
|
|
|
contains: "## Outcome"
|
|
|
|
|
key_links:
|
|
|
|
|
- from: ../sm-demo-app/plugins/golem15/user/controllers/api_controller.go (Login)
|
|
|
|
|
to: summercms.go modules/surf/clientip.go (ClientIP, TrustedProxies)
|
|
|
|
|
via: "ip := surf.ClientIP(r, surf.TrustedProxies(appConfig(app))), read per request, feeds RejectIfThrottled, refuseLogin and CheckAndRecordLogin"
|
|
|
|
|
pattern: "surf\\.ClientIP\\(r, surf\\.TrustedProxies\\(appConfig\\(app\\)\\)\\)"
|
|
|
|
|
- from: ../sm-demo-app/plugins/golem15/user/controllers/api_controller.go (ResetPassword) and controllers/users_admin_controller.go (FormBeforeUpdate)
|
|
|
|
|
to: summercms.go modules/bouncer/jwt.go (issuedBeforeCutoff), controllers/api_controller.go refresh check (:286) and classes/session_user.go RequestUser (:83)
|
|
|
|
|
via: "all three compare iat.Before(tokens_valid_after); iat is whole seconds (bouncer.Mint uses jwt.NewNumericDate), so a whole-second cutoff accepts a token minted in the same second"
|
|
|
|
|
pattern: "Truncate\\(time\\.Second\\)"
|
|
|
|
|
- from: Task 1 and Task 2 commit SHAs (sm-user-plugin master)
|
|
|
|
|
to: .planning/notes/2026-10-22-external-user-plugin-escalations.md rows 9 and 10 and reply paragraphs
|
|
|
|
|
via: "Task 3 reads each SHA with git log --format=%h --grep on the commit subject"
|
|
|
|
|
pattern: "→ v0\\.5\\.0"
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
<objective>
|
|
|
|
|
Close external escalation items 9 and 10 in sm-user-plugin.
|
|
|
|
|
|
|
|
|
|
Item 9: POST /login keys its failed-login throttle on the host of r.RemoteAddr. Behind nginx or Caddy every visitor has the proxy's address, so five wrong passwords lock the account for everyone for 15 minutes. /register and the user-api rate limit already use surf.ClientIP with http.trusted_proxies. /login will do the same. The same `ip` variable also feeds RejectIfThrottled and refuseLogin (G2's activation refusal), so one assignment fixes all three. Exported controllers.RemoteIP stays as the RemoteAddr host helper (social_hooks_test uses it, and it is public API of a core plugin). Only its doc comment changes.
|
|
|
|
|
|
|
|
|
|
Item 10: JWT iat is whole seconds. Every cutoff check is iat.Before(tokens_valid_after). RevokeUserAccess already stamps the cutoff truncated to the second. /reset-password and the admin Users password set stamp sub-second time, so a token minted later in the same second is refused, intermittently. Both writers will truncate down. /change-password (iat minus one second) and bouncer's comparison stay as they are.
|
|
|
|
|
|
|
|
|
|
Purpose: one attacker behind a shared proxy cannot lock everyone else out of an account, and a login right after a password reset no longer gets 401 at random.
|
|
|
|
|
Output: two local fix commits on sm-user-plugin master (one logical change each, per CLAUDE.md), made in /media/nvme/dev/golem15/summercms.io/summercms/sm-demo-app/plugins/golem15/user with git -C. Then the escalation note and the todo in summercms.go are filled and left uncommitted. No push, no tag and no sm-demo-app pointer bump, because of the SummerCMS v0.5.0 release hold. summercms.go code (modules/bouncer, modules/surf) is not modified.
|
|
|
|
|
|
|
|
|
|
Planner's choices (quick task, no CONTEXT.md; the executor does not revisit them):
|
|
|
|
|
- QD-1 Two commits, not one. CLAUDE.md says one logical change per commit, and the throttle key and the cutoff precision are separate fixes. Each escalation row and reply paragraph then names its own SHA.
|
|
|
|
|
- QD-2 /login uses exactly /register's expression, surf.ClientIP(r, surf.TrustedProxies(appConfig(app))), read per request, so a config value set after Boot (as the harness's configure functions do) applies. The unexported remoteIP stays because RemoteIP still calls it.
|
|
|
|
|
- QD-3 In /reset-password only the tokens_valid_after value is truncated (now.Truncate(time.Second), inline in the Updates map). email_verified_at keeps `now` (D-27 address proof, sub-second precision is harmless there). In FormBeforeUpdate, `now` feeds only TokensValidAfter, so the assignment itself becomes time.Now().Truncate(time.Second).
|
|
|
|
|
- QD-4 The tests go through the real router (newAdminEnv: surf.Assemble with the jwt guard, the user-api limiter and the admin API), so the throttle test sees the request as nginx would deliver it and the cutoff test covers all three comparison sites (bouncer's jwt guard on GET /tokens, /fetch, /refresh). The same-second proof is deterministic: the stored cutoff is checked to be a whole second, and tokens are crafted with iat equal to the cutoff (accepted) and one second earlier (refused) using userAudienceToken. A real login right after the reset is also checked, after aligning to a fresh second, but whether it falls in the same second is only logged, never asserted, because a slow run would make that flaky.
|
|
|
|
|
- QD-5 README: only behaviour that names the /login throttle key changes: the Login throttling bullet, the RemoteIP clause, and one Testing paragraph in the throttle commit. The cutoff commit touches no README text. No documented behaviour states the reset cutoff's precision; the RevokeUserAccess entry already says "before the current second".
|
|
|
|
|
|
|
|
|
|
Source coverage audit:
|
|
|
|
|
| Source | Item | Covered by |
|
|
|
|
|
|--------|------|------------|
|
|
|
|
|
| GOAL / todo Solution | /login throttle IP = surf.ClientIP with trusted proxies, same as /register | Task 1 |
|
|
|
|
|
| todo Solution | refuseLogin records the same ip (one assignment) | Task 1 (same `ip` variable; TestLoginThrottleClientIP counts refusals on the XFF row) |
|
|
|
|
|
| todo Solution | exported RemoteIP stays; doc comment no longer claims /login | Task 1 |
|
|
|
|
|
| todo Solution | /reset-password and admin password set truncate the cutoff; change-password and bouncer untouched | Task 2 (code + grep gates + bouncer diff check) |
|
|
|
|
|
| todo Tests | XFF behind a trusted proxy is the /login throttle key | Task 1 TestLoginThrottleClientIP |
|
|
|
|
|
| todo Tests | reset (and admin password set) then a same-second token succeeds | Task 2 TestPasswordCutoffSameSecond |
|
|
|
|
|
| todo / orchestrator | hold push/tag (v0.5.0); base 4bf44fc | Tasks 1-2 commit gates |
|
|
|
|
|
| orchestrator | fill items 9 and 10 (tracker, reply), todo Outcome, stays in pending, not committed | Task 3 |
|
|
|
|
|
| orchestrator | README only if the throttle key is documented | Task 1 (it is: README :23 and :183) |
|
|
|
|
|
| REQ / RESEARCH / CONTEXT | none (quick task, no research, no discuss) | n/a |
|
|
|
|
|
</objective>
|
|
|
|
|
|
|
|
|
|
<execution_context>
|
|
|
|
|
@~/.claude/msd-core/workflows/execute-plan.md
|
|
|
|
|
@~/.claude/msd-core/templates/summary.md
|
|
|
|
|
</execution_context>
|
|
|
|
|
|
|
|
|
|
<context>
|
|
|
|
|
@CLAUDE.md
|
|
|
|
|
@.planning/STATE.md
|
|
|
|
|
@.planning/todos/pending/2026-10-22-user-login-lockout-and-same-second-cutoff.md
|
|
|
|
|
@.planning/notes/2026-10-22-external-user-plugin-escalations.md
|
|
|
|
|
@../sm-demo-app/plugins/golem15/user/controllers/api_controller.go
|
|
|
|
|
@modules/surf/clientip.go
|
|
|
|
|
|
|
|
|
|
Working copies (observed at planning time, 2026-10-22):
|
|
|
|
|
- sm-user-plugin is at ../sm-demo-app/plugins/golem15/user (absolute /media/nvme/dev/golem15/summercms.io/summercms/sm-demo-app/plugins/golem15/user). Branch master at 4bf44fc (G4), 3 ahead of origin/master, working tree clean, no tag on HEAD. Do not fetch, merge, rebase or reset. If master has moved past 4bf44fc, build on top and record `git log --oneline 4bf44fc..master` in the SUMMARY. If the tree is dirty, stop: another session is working there.
|
|
|
|
|
- Go: sm-demo-app's go.work does not list the plugin under `use`, so every go command in the plugin dir runs with GOWORK=off. go.mod:176 replaces summercms with ../../../../summercms.go. sm-demo-app's go.work replaces sm-user-plugin with this checkout for the sociallogin and oauthserver consumers.
|
|
|
|
|
- Docker: docker info answered (server 29.9.1). If the socket is denied, run go test through sudo -u golem (CLAUDE.md).
|
|
|
|
|
- sm-demo-app (../sm-demo-app) has another session's uncommitted edits (main.go and the plugins/golem15/user pointer). It is read-only here: run tests, never stage or commit.
|
|
|
|
|
- summercms.go has other sessions' uncommitted edits (.planning/state.json, untracked .msd/ and phase .gitkeep files). Leave them alone. STATE.md, g15office and every commit in summercms.go belong to the orchestrator.
|
|
|
|
|
|
|
|
|
|
Source facts (plugin checkout at 4bf44fc):
|
|
|
|
|
- controllers/api_controller.go:
|
|
|
|
|
- Login doc comment :46-57 and handler :58-140. `ip := remoteIP(r)` :77 feeds RejectIfThrottled :79, refuseLogin :89 and :96 (the G2 activation gate), and CheckAndRecordLogin :107.
|
|
|
|
|
- Register :746 already has `ip := surf.ClientIP(r, surf.TrustedProxies(appConfig(app)))`. surf is already imported; appConfig :883 is nil-safe.
|
|
|
|
|
- ResetPassword :373-430. `now := time.Now()` :418 feeds both `"tokens_valid_after": now` :422 and `"email_verified_at": now` :423 in one Updates map. gofmt aligns those keys; "tokens_valid_after" is the longest, so it takes one space after its colon.
|
|
|
|
|
- ChangePassword cutoff `cutoff := iat.Add(-time.Second)` :674, written at :679 and :686. Leave it alone.
|
|
|
|
|
- Refresh-side cutoff check `iat.Time.Before(*user.TokensValidAfter)` :286.
|
|
|
|
|
- RemoteIP doc comment :1162-1164 (it claims /login's throttle uses it), RemoteIP :1165-1167, remoteIP :1169-1178 (returns raw RemoteAddr when it has no port).
|
|
|
|
|
- controllers/users_admin_controller.go: FormBeforeUpdate :216-249. When storePassword reports a change, :243-246 runs `now := time.Now()` and `user.TokensValidAfter = &now`; cabana's Save writes it.
|
|
|
|
|
- classes/admin_actions.go RevokeUserAccess :106-120, `cutoff := time.Now().Truncate(time.Second)` :113 (the reference). classes/session_user.go :83 RequestUser cutoff check.
|
|
|
|
|
- classes/throttle.go: rows are per (user_id, ip_address). RejectIfThrottled and CheckAndRecordLogin select `user_id = ? AND (ip_address = ? OR ip_address IS NULL)`. A missing row is created with IPAddress &ip on the first recorded attempt, success included.
|
|
|
|
|
- summercms.go (read-only): surf.ClientIP uses RemoteAddr unless it lies inside a trusted prefix; then it takes the rightmost X-Forwarded-For hop outside the trusted prefixes. It returns "" for an unparseable RemoteAddr and unmaps IPv4-mapped IPv6. surf.TrustedProxies reads http.trusted_proxies ([]string or []any of CIDRs). bouncer.Mint sets IssuedAt with jwt.NewNumericDate (whole seconds). issuedBeforeCutoff is `iat.Before(user.TokensValidAfter)` (jwt.go:172-174).
|
|
|
|
|
- Plugin.Buckets (plugin.go:146-166): the user-api limiter (120/min) keys on the verified bearer sub or on surf.ClientIP with the trusted proxies read at Assemble.
|
|
|
|
|
- Tests (package user, plugin root):
|
|
|
|
|
- admin_harness_test.go: adminHarnessDB :91 (skips under -short); newAdminEnv(t, configure...) :165. Configure functions run after party.Activate and before Assemble, and each calls cfg.Set and t.Fatal on error (see admin_groups_edge_test.go:120).
|
|
|
|
|
- adminEnv: call/send :287/:301. send sets Host example.com, Accept-Language en and Content-Type, and gives every request its own RemoteAddr 198.51.100.N:4000, so it cannot model a fixed proxy. Also expect :416, seedUser :513 (password correct-horse, unique email), reload :554, admin(t, perms...) :242, userLogin :682, userPath(id, suffix) :507, adminStamp :85.
|
|
|
|
|
- logout_test.go: guarded(t, token, cookie) :220 (GET /_user/api/v1/tokens, jwt.auth) and userAudienceToken(t, sub, jti, iat, exp) :291 (signs like bouncer.Mint).
|
|
|
|
|
- offboarding_test.go: fetchCode :45, refresh :50, assertRefreshRefused :55.
|
|
|
|
|
- session_test.go: invalidLoginBody, parseToken :644. session_cookie_test.go: userSub :237.
|
|
|
|
|
- Existing coverage that must stay green: TestLogin (seeds a suspended row at 192.0.2.1, httptest's default address, and expects the block), TestLoginSixthAttempt, TestLoginRequiresActivation, TestRegistrationExports (RemoteIP with and without a port; X-Forwarded-For ignored), TestResetPassword, TestAdminUserPassword and TestAdminPrivilegedMember T-12.1-21 (both sleep 1.1s and check an older token is refused), TestRevokeUserAccess in classes (checks the whole-second cutoff).
|
|
|
|
|
- README.md: the Login throttling bullet :23 ("Failed logins are counted per user and IP in `user_throttle` ..."). The `RefreshTTL`/`RemoteIP` clause :183 says /login keys its throttle on RemoteIP. The social-login helpers list :43 names RemoteIP (keep it). Testing section :247-272; the TestLoginRequiresActivation paragraph is :268. README does not mention http.trusted_proxies today.
|
|
|
|
|
- The names TestLoginThrottleClientIP, TestPasswordCutoffSameSecond, loginFrom and waitForFreshSecond, and the files login_client_ip_test.go and same_second_cutoff_test.go, are all unused today.
|
|
|
|
|
- Escalation note (summercms.go): tracker rows 9 and 10 at :30-31 (status open, Shipped empty). Rows 1-3 :22-24 are the format. Verdicts **9.** :63 and **10.** :64; G3/G4 :56-57 are the "Done locally (`sha`). ..." format. Reply TODO lines :104 (under "Login lockout keyed on RemoteAddr") and :108 (under "Same-second token refusal"). G2-G4 "Ships in SummerCMS v0.5.0 (sm-user-plugin <sha>, tagged with that wave): ..." lines :75, :79, :83 are the format. The note has four lines starting with TODO today; the two for items 7 and 8 stay.
|
|
|
|
|
- Todo: the Outcome section goes between "## Solution" and "## Invoke", as in .planning/todos/pending/2026-10-22-user-logout-blacklist-missing-under-serve.md:33-35.
|
|
|
|
|
</context>
|
|
|
|
|
|
|
|
|
|
<tasks>
|
|
|
|
|
|
|
|
|
|
<task type="tracer" tdd="true">
|
|
|
|
|
<name>Task 1 (tracer): /login throttles per client address behind a trusted proxy, end to end through the router on Postgres, committed in sm-user-plugin</name>
|
|
|
|
|
<files>../sm-demo-app/plugins/golem15/user/controllers/api_controller.go, ../sm-demo-app/plugins/golem15/user/login_client_ip_test.go, ../sm-demo-app/plugins/golem15/user/README.md</files>
|
|
|
|
|
<precondition>git -C /media/nvme/dev/golem15/summercms.io/summercms/sm-demo-app/plugins/golem15/user status --short is empty, its branch is master, and 4bf44fc is an ancestor of HEAD. Docker is reachable for testcontainers, directly or through sudo -u golem.</precondition>
|
|
|
|
|
<behavior>
|
|
|
|
|
- TestLoginThrottleClientIP (login_client_ip_test.go, package user, Docker through newAdminEnv; skips under -short). Two subtests, each with its own env and a fresh seedUser account (password correct-horse).
|
|
|
|
|
- Subtest "behind a trusted proxy": newAdminEnv configured with http.trusted_proxies = []string{"10.0.0.0/8"}. All requests come from RemoteAddr 10.0.0.2:5000.
|
|
|
|
|
- Five wrong-password POSTs to /_user/api/v1/login with X-Forwarded-For 203.0.113.10 each answer 401 with invalidLoginBody.
|
|
|
|
|
- The account then has exactly one user_throttle row: ip_address 203.0.113.10, attempts 5, is_suspended true.
|
|
|
|
|
- The correct password with X-Forwarded-For 203.0.113.10 answers 401 invalidLoginBody.
|
|
|
|
|
- The correct password with X-Forwarded-For 203.0.113.20 through the same proxy answers 200 with a non-empty token.
|
|
|
|
|
- Subtest "untrusted X-Forwarded-For is ignored": newAdminEnv with no http.trusted_proxies. All requests come from RemoteAddr 192.0.2.50:4000.
|
|
|
|
|
- Five wrong-password POSTs, carrying X-Forwarded-For 203.0.113.1 through 203.0.113.5 in turn, each answer 401.
|
|
|
|
|
- Exactly one row exists, keyed 192.0.2.50, with attempts 5 and suspended.
|
|
|
|
|
- The correct password with X-Forwarded-For 203.0.113.99 from 192.0.2.50 answers 401 invalidLoginBody.
|
|
|
|
|
- The correct password from RemoteAddr 192.0.2.51:4000 answers 200: the throttle is still per address without proxies.
|
|
|
|
|
- RED at 4bf44fc: the trusted-proxy subtest fails at the row assertion, because the row is keyed 10.0.0.2. The untrusted subtest already passes at 4bf44fc and pins the anti-spoofing property against a future blind trust of X-Forwarded-For.
|
|
|
|
|
</behavior>
|
|
|
|
|
<action>
|
|
|
|
|
RED. Write login_client_ip_test.go (package user) with TestLoginThrottleClientIP as the behavior block describes. Its doc comment says it pins the /login throttle key: the trusted-proxy client address, as /register and the user-api limit use, and never an X-Forwarded-For header from an untrusted peer.
|
|
|
|
|
|
|
|
|
|
Add the helper (e *adminEnv) loginFrom(t *testing.T, remoteAddr, forwardedFor, email, password string) *httptest.ResponseRecorder in the same file. It builds a POST /_user/api/v1/login request the way send does: JSON body {"email","password"}, Host example.com, Accept-Language en, Content-Type application/json. It sets req.RemoteAddr = remoteAddr, sets X-Forwarded-For only when forwardedFor is non-empty, and serves it through e.h. A short doc comment explains why send cannot be used: send gives every request its own address.
|
|
|
|
|
|
|
|
|
|
The trusted-proxy subtest's configure function calls cfg.Set("http.trusted_proxies", []string{"10.0.0.0/8"}) and calls t.Fatal on error, the same way admin_groups_edge_test.go:120 does. Read the rows with e.gdb.Where("user_id = ?", user.ID).Find into []models.Throttle, and compare *IPAddress (it is a pointer). Check the rows before the 203.0.113.20 login, because a successful login creates that client's own row.
|
|
|
|
|
|
|
|
|
|
Run the test. It must fail against the unchanged handler; record the failure line for the SUMMARY. Do not commit a red test.
|
|
|
|
|
|
|
|
|
|
GREEN, per QD-2. In controllers/api_controller.go Login, replace the :77 assignment with ip := surf.ClientIP(r, surf.TrustedProxies(appConfig(app))), the expression Register uses at :746. Leave RejectIfThrottled, refuseLogin and CheckAndRecordLogin as they are; they already take `ip`.
|
|
|
|
|
|
|
|
|
|
Extend the Login doc comment by one sentence: the throttle counts failed attempts per user and client address, which is surf.ClientIP with http.trusted_proxies, as for /register.
|
|
|
|
|
|
|
|
|
|
Rewrite the RemoteIP doc comment. It should say that RemoteIP returns the host part of r.RemoteAddr (r.RemoteAddr itself when it has no port, "" when it is empty), with no proxy handling. It should also say that /login and /register key their throttles on surf.ClientIP with the trusted proxies, and that RemoteIP stays for callers that want the connection address. Keep the RemoteIP and remoteIP code unchanged. gofmt the file.
|
|
|
|
|
|
|
|
|
|
README.md, in the same commit (per QD-5):
|
|
|
|
|
- Rewrite the Login throttling bullet (:23) so it says what follows. Failed logins are counted per user and client address in `user_throttle` (`CheckAndRecordLogin`, `RejectIfThrottled`) and suspend the account for a configured time. The client address is `surf.ClientIP` with the framework's `http.trusted_proxies`, as for the `/register` IP limit and the `user-api` rate limit. Behind a listed reverse proxy it is the `X-Forwarded-For` client, otherwise the connection address, and an `X-Forwarded-For` header from any other peer is ignored. A deployment behind a proxy lists the proxy's address in `http.trusted_proxies`; otherwise every visitor shares the proxy's counter.
|
|
|
|
|
- At :183, change the RemoteIP half of the sentence to: `RemoteIP(r)` the host part of `r.RemoteAddr`, with no proxy handling (`/login` and `/register` use `surf.ClientIP` with `http.trusted_proxies`).
|
|
|
|
|
- After the TestLoginRequiresActivation paragraph (:268), add a paragraph. `TestLoginThrottleClientIP` sends failed logins through one trusted proxy (`http.trusted_proxies`). It checks that the throttle row carries the `X-Forwarded-For` client, and that this client is suspended while another client behind the same proxy still signs in. It also checks that without trusted proxies a changing `X-Forwarded-For` neither moves the counter nor escapes the suspension.
|
|
|
|
|
|
|
|
|
|
Run go vet, the targeted tests, then the full Docker-backed suite of the plugin, so the commit is green (CLAUDE.md). If an existing test fails because it relied on the raw RemoteAddr key, stop and report it; planning found none. A failure unrelated to this change is not patched here: stop and report it.
|
|
|
|
|
|
|
|
|
|
Commit in the plugin repo only, staging by path. Run git -C /media/nvme/dev/golem15/summercms.io/summercms/sm-demo-app/plugins/golem15/user add controllers/api_controller.go login_client_ip_test.go README.md, then git -C with the same path and commit with this subject:
|
|
|
|
|
fix: /login throttles failed logins per client address behind trusted proxies (surf.ClientIP), like /register
|
|
|
|
|
The body is one short paragraph: behind a reverse proxy every visitor had the proxy's RemoteAddr, so one client's failures locked the account for all; the key is now surf.ClientIP with http.trusted_proxies; RemoteIP is unchanged. Add no trailer of any kind; the user's global CLAUDE.md forbids co-author tags and overrides any harness attribution reminder. Do not push or tag (v0.5.0 release hold). Record the short SHA (git log -1 --format=%h) and the RED line in the SUMMARY.
|
|
|
|
|
</action>
|
|
|
|
|
<verify>
|
|
|
|
|
<automated>cd /media/nvme/dev/golem15/summercms.io/summercms/sm-demo-app/plugins/golem15/user && test -z "$(gofmt -l controllers/api_controller.go login_client_ip_test.go)" && GOWORK=off go vet ./... && GOWORK=off go test -count=1 -run '^(TestLoginThrottleClientIP|TestLogin|TestLoginSixthAttempt|TestLoginRequiresActivation|TestRegistrationExports)$' . && L=$(awk '/^func Login\(/,/^}/' controllers/api_controller.go) && grep -qF 'ip := surf.ClientIP(r, surf.TrustedProxies(appConfig(app)))' <<<"$L" && ! grep -qF 'remoteIP(r)' <<<"$L" && G=$(awk '/^func Register\(/,/^}/' controllers/api_controller.go) && grep -qF 'ip := surf.ClientIP(r, surf.TrustedProxies(appConfig(app)))' <<<"$G" && R=$(awk '/^func RemoteIP\(/,/^}/' controllers/api_controller.go) && grep -qF 'return remoteIP(r)' <<<"$R" && grep -qF 'func RemoteIP(r *http.Request) string' controllers/api_controller.go && ! grep -qF 'address /login uses' controllers/api_controller.go && grep -qF 'with no proxy handling' controllers/api_controller.go && ! grep -qF 'uses for its throttle' README.md && grep -qF 'http.trusted_proxies' README.md && grep -qF 'TestLoginThrottleClientIP' README.md && grep -qF 'func TestLoginThrottleClientIP' login_client_ip_test.go && grep -qF 'func (e *adminEnv) loginFrom' login_client_ip_test.go && GOWORK=off go test -count=1 ./... && test -z "$(git status --short)" && C=$(git log -1 --format=%H --fixed-strings --grep='per client address behind trusted proxies' master) && test -n "$C" && B=$(git log -1 --format=%B "$C") && ! grep -qi 'co-authored-by' <<<"$B" && git diff --quiet "$C^" "$C" -- go.mod go.sum && F=$(git diff --name-only "$C^" "$C") && test "$(LC_ALL=C sort <<<"$F" | tr '\n' ' ')" = "README.md controllers/api_controller.go login_client_ip_test.go " && TG=$(git tag --points-at "$C") && test -z "$TG" && RB=$(git branch -r --contains "$C") && test -z "$RB"</automated>
|
|
|
|
|
</verify>
|
|
|
|
|
<done>TestLoginThrottleClientIP failed at 4bf44fc (failure line recorded) and now passes. Behind a trusted proxy the X-Forwarded-For client is the throttle key, so one client's lockout does not lock another client behind the same proxy, and without trusted proxies X-Forwarded-For is ignored. RemoteIP is unchanged in code, with a corrected doc comment. The README describes the client-address rule, http.trusted_proxies and the new test. go vet and the full plugin suite are green. One fix commit (api_controller.go, login_client_ip_test.go, README.md) is on sm-user-plugin master: no trailer, go.mod and go.sum unchanged, untagged, not on any remote branch. The tree is clean.</done>
|
|
|
|
|
</task>
|
|
|
|
|
|
|
|
|
|
<task type="auto" tdd="true">
|
|
|
|
|
<name>Task 2: reset-password and the admin password set stamp a whole-second cutoff, so a same-second token is accepted; full suites, consumer cross-check, commit</name>
|
|
|
|
|
<files>../sm-demo-app/plugins/golem15/user/controllers/api_controller.go, ../sm-demo-app/plugins/golem15/user/controllers/users_admin_controller.go, ../sm-demo-app/plugins/golem15/user/same_second_cutoff_test.go</files>
|
|
|
|
|
<precondition>Task 1's verify passed: its commit is HEAD of sm-user-plugin master and the plugin tree is clean.</precondition>
|
|
|
|
|
<behavior>
|
|
|
|
|
- TestPasswordCutoffSameSecond (same_second_cutoff_test.go, package user, Docker through one newAdminEnv; skips under -short) has two subtests, each with a fresh seedUser account.
|
|
|
|
|
- Subtest "reset-password":
|
|
|
|
|
- Issue a code with classes.IssueResetCode(t.Context(), e.gdb, user), call waitForFreshSecond(t), then POST /_user/api/v1/reset-password through e.call with code "<id>!<code>" and a new password plus its confirmation. It answers 200 {"message":"Password has been reset"}.
|
|
|
|
|
- e.reload(t, user.ID): TokensValidAfter is non-nil and Equal to its own Truncate(time.Second), and EmailVerifiedAt is non-nil.
|
|
|
|
|
- Subtest "admin password set": an admin token from e.admin(t, permAccessUsers); waitForFreshSecond(t); then PUT userPath(user.ID, "") with name, email, password and password_confirmation, which answers 200 through e.expect. TokensValidAfter after reload is non-nil and a whole second.
|
|
|
|
|
- Both subtests then run the same checks with cutoff := the reloaded TokensValidAfter:
|
|
|
|
|
- a token from userAudienceToken(t, userSub(user), a fresh jti, cutoff, cutoff plus one hour) gets 200 from e.guarded(t, tok, "") (bouncer's jwt guard), 200 from e.fetchCode, and 200 from e.refresh (refresh last, because it revokes);
|
|
|
|
|
- a token with iat cutoff minus one second gets 401 from guarded and fetchCode, and assertRefreshRefused passes on e.refresh;
|
|
|
|
|
- e.userLogin with the new password answers 200, and that token gets 200 from guarded and fetchCode. Whether its iat (parseToken) equals the cutoff second is only logged with t.Logf (QD-4).
|
|
|
|
|
- RED at the Task 1 commit: both subtests fail at the whole-second assertion, because the stored cutoff carries microseconds.
|
|
|
|
|
</behavior>
|
|
|
|
|
<action>
|
|
|
|
|
RED. Write same_second_cutoff_test.go (package user) with TestPasswordCutoffSameSecond as the behavior block describes. Its doc comment says it pins the whole-second tokens_valid_after cutoff of POST /reset-password and of the admin Users password set: JWT iat has one-second resolution, so a token minted in the second of the reset is accepted on every check site and an earlier second is refused.
|
|
|
|
|
|
|
|
|
|
Add waitForFreshSecond(t *testing.T) in the same file. It sleeps until 10ms after the next whole second, so that the reset, the reload and the login usually fall within one second. Its comment says the deterministic proof is the crafted-token pair, not this alignment.
|
|
|
|
|
|
|
|
|
|
Use fresh jtis built from adminStamp(). Reuse the existing helpers; add no new harness code outside this file. Run the test. It must fail against the unchanged writers; record the failure line for the SUMMARY. Do not commit a red test.
|
|
|
|
|
|
|
|
|
|
GREEN, per QD-3. In controllers/api_controller.go ResetPassword:
|
|
|
|
|
- keep now := time.Now();
|
|
|
|
|
- change only the map entry to "tokens_valid_after": now.Truncate(time.Second), and leave "email_verified_at" as now;
|
|
|
|
|
- extend the comment above the write by one sentence: the session cutoff is truncated to whole seconds, as in RevokeUserAccess, because JWT iat has whole-second precision, so the login that follows the reset is not refused.
|
|
|
|
|
|
|
|
|
|
In controllers/users_admin_controller.go FormBeforeUpdate, change the assignment to now := time.Now().Truncate(time.Second) and add the same reason to the comment above it.
|
|
|
|
|
|
|
|
|
|
Do not touch ChangePassword's cutoff := iat.Add(-time.Second), RevokeUserAccess, or anything in summercms.go modules/bouncer. gofmt both files.
|
|
|
|
|
|
|
|
|
|
Run go vet, the targeted tests, TestRevokeUserAccess in ./classes, go test -short and the full Docker-backed suite of the plugin.
|
|
|
|
|
|
|
|
|
|
Consumer cross-check, read-only: from /media/nvme/dev/golem15/summercms.io/summercms/sm-demo-app run go test -count=1 ./plugins/golem15/sociallogin/... ./plugins/golem15/oauthserver/... (go.work points them at this checkout, which carries both fix commits). Record the result in the SUMMARY. If a failure names /login, the throttle, RemoteIP, tokens_valid_after or the reset/admin password path of the user plugin, stop and fix it in this task. Record any other failure as unrelated; do not patch those repos. Do not stage anything in sm-demo-app.
|
|
|
|
|
|
|
|
|
|
Commit in the plugin repo only, staging by path. Run git -C /media/nvme/dev/golem15/summercms.io/summercms/sm-demo-app/plugins/golem15/user add controllers/api_controller.go controllers/users_admin_controller.go same_second_cutoff_test.go, then git -C with the same path and commit with this subject:
|
|
|
|
|
fix: reset-password and the admin password set stamp tokens_valid_after in whole seconds, so a token minted in the same second is accepted
|
|
|
|
|
The body is one short paragraph: iat is whole seconds and the checks are iat before cutoff, so a sub-second cutoff refused the next login in that second; both writers now truncate like RevokeUserAccess; change-password and bouncer are unchanged. No trailer, no push, no tag, no pointer bump. Record the short SHA and the test and cross-check results in the SUMMARY.
|
|
|
|
|
</action>
|
|
|
|
|
<verify>
|
|
|
|
|
<automated>cd /media/nvme/dev/golem15/summercms.io/summercms/sm-demo-app/plugins/golem15/user && test -z "$(gofmt -l controllers/api_controller.go controllers/users_admin_controller.go same_second_cutoff_test.go)" && GOWORK=off go vet ./... && GOWORK=off go test -count=1 -run '^(TestPasswordCutoffSameSecond|TestResetPassword|TestAdminUserPassword|TestAdminPrivilegedMember|TestSessionRoutesAcceptCookies|TestOffboarding|TestRefreshSubjectChecks)$' . && GOWORK=off go test -count=1 -run '^TestRevokeUserAccess$' ./classes && grep -qF '"tokens_valid_after": now.Truncate(time.Second),' controllers/api_controller.go && grep -qE '"email_verified_at": +now,' controllers/api_controller.go && grep -qF 'cutoff := iat.Add(-time.Second)' controllers/api_controller.go && grep -qF 'now := time.Now().Truncate(time.Second)' controllers/users_admin_controller.go && grep -qF 'func TestPasswordCutoffSameSecond' same_second_cutoff_test.go && grep -qF 'func waitForFreshSecond' same_second_cutoff_test.go && GOWORK=off go test -short -count=1 ./... && GOWORK=off go test -count=1 ./... && test -z "$(git status --short)" && C=$(git log -1 --format=%H --fixed-strings --grep='stamp tokens_valid_after in whole seconds' master) && test -n "$C" && test "$C" = "$(git rev-parse master)" && test "$(git rev-parse "$C^")" = "$(git log -1 --format=%H --fixed-strings --grep='per client address behind trusted proxies' master)" && B=$(git log -1 --format=%B "$C") && ! grep -qi 'co-authored-by' <<<"$B" && git diff --quiet "$C^" "$C" -- go.mod go.sum README.md && F=$(git diff --name-only "$C^" "$C") && test "$(LC_ALL=C sort <<<"$F" | tr '\n' ' ')" = "controllers/api_controller.go controllers/users_admin_controller.go same_second_cutoff_test.go " && TG=$(git tag --points-at "$C") && test -z "$TG" && RB=$(git branch -r --contains "$C") && test -z "$RB" && git -C /media/nvme/dev/golem15/summercms.io/summercms/summercms.go diff --quiet HEAD -- modules/bouncer modules/surf</automated>
|
|
|
|
|
</verify>
|
|
|
|
|
<done>TestPasswordCutoffSameSecond failed at the Task 1 commit (failure line recorded) and now passes. After a reset-password and after an admin password set, tokens_valid_after is a whole second. A token issued in that second is accepted by the jwt guard, /fetch and /refresh; one second earlier is refused at all three; the login right after works; email_verified_at is still written. Change-password, RevokeUserAccess and bouncer are untouched. The plugin's go vet, go test -short and full suite are green, and the sociallogin/oauthserver cross-check result is recorded. A second fix commit (api_controller.go, users_admin_controller.go, same_second_cutoff_test.go) sits on top of Task 1's: no trailer, go.mod, go.sum and README unchanged, untagged, unpushed. The tree is clean.</done>
|
|
|
|
|
</task>
|
|
|
|
|
|
|
|
|
|
<task type="auto">
|
|
|
|
|
<name>Task 3: fill items 9 and 10 in the escalation note and the todo's Outcome (summercms.go, left uncommitted)</name>
|
|
|
|
|
<files>.planning/notes/2026-10-22-external-user-plugin-escalations.md, .planning/todos/pending/2026-10-22-user-login-lockout-and-same-second-cutoff.md</files>
|
|
|
|
|
<precondition>Task 2's verify passed: sm-user-plugin master carries both fix commits.</precondition>
|
|
|
|
|
<action>
|
|
|
|
|
Get the two short SHAs from /media/nvme/dev/golem15/summercms.io/summercms/sm-demo-app/plugins/golem15/user:
|
|
|
|
|
- S9 = git log -1 --format=%h --fixed-strings --grep='per client address behind trusted proxies' master
|
|
|
|
|
- S10 = git log -1 --format=%h --fixed-strings --grep='stamp tokens_valid_after in whole seconds' master
|
|
|
|
|
|
|
|
|
|
Edit .planning/notes/2026-10-22-external-user-plugin-escalations.md with Edit (scoped replacements only, never a whole-file Write):
|
|
|
|
|
- Tracker row 9, using rows 1-3 as the format. The Quick / todo cell becomes [261022-vfz](../quick/261022-vfz-user-plugin-login-throttle-via-surf-clie/) · [todo](../todos/pending/2026-10-22-user-login-lockout-and-same-second-cutoff.md). Status becomes done (unreleased). Shipped becomes `S9` → v0.5.0. Label, Verdict and g15office stay.
|
|
|
|
|
- Tracker row 10: the same cell links, done (unreleased), and `S10` → v0.5.0.
|
|
|
|
|
- Verdicts, in the G3/G4 format:
|
|
|
|
|
- **9.** becomes: Done locally (`S9`). /login keys the throttle on surf.ClientIP with http.trusted_proxies (the /register expression); RemoteIP stays the RemoteAddr helper; without trusted proxies X-Forwarded-For is ignored.
|
|
|
|
|
- **10.** becomes: Done locally (`S10`). /reset-password and the admin password set truncate the cutoff to whole seconds like RevokeUserAccess; change-password and bouncer unchanged.
|
|
|
|
|
- Reply to send, "Login lockout keyed on RemoteAddr": keep the "Agreed. ..." line. Replace the TODO line under it with one line in the G2-G4 format: Ships in SummerCMS v0.5.0 (sm-user-plugin S9, tagged with that wave): /login counts failed logins per account and client address, and the client address is now the one /register and the user-api rate limit use: behind a proxy listed in http.trusted_proxies, the X-Forwarded-For client; otherwise the connection address. An X-Forwarded-For header from an address not in http.trusted_proxies is ignored, so it cannot dodge or move a lockout. List your nginx/Caddy address in http.trusted_proxies; without it every visitor still counts as the proxy.
|
|
|
|
|
- Reply to send, "Same-second token refusal": keep the "Agreed. ..." line and replace the TODO line under it with: Ships in SummerCMS v0.5.0 (sm-user-plugin S10, tagged with that wave): POST /reset-password and an administrator's password set on the Users form stamp tokens_valid_after with the current whole second, as deactivate and ban already do, so a token minted in the same second as the reset (the login right after it) is accepted and every token from an earlier second is still refused. As with deactivate and ban, a token issued earlier within that same second also stays valid, because iat has no finer resolution. /change-password and the guard's comparison are unchanged.
|
|
|
|
|
- In both reply lines, substitute the real S9 and S10, unquoted, as G2-G4 do. Leave the two TODO lines for items 7 and 8, and everything else in the note, unchanged.
|
|
|
|
|
|
|
|
|
|
Edit the todo .planning/todos/pending/2026-10-22-user-login-lockout-and-same-second-cutoff.md: insert an "## Outcome (2026-10-22)" section between "## Solution" (after its last paragraph) and "## Invoke", in the format of 2026-10-22-user-logout-blacklist-missing-under-serve.md. One paragraph, which:
|
|
|
|
|
- names quick `261022-vfz`, the two fix commits `S9` and `S10` on sm-user-plugin master and their base `4bf44fc`;
|
|
|
|
|
- describes S9: /login uses surf.ClientIP(r, surf.TrustedProxies(appConfig(app))) as /register does; the same ip feeds RejectIfThrottled, refuseLogin and CheckAndRecordLogin; exported RemoteIP is unchanged with a corrected doc comment; README updated; TestLoginThrottleClientIP (Docker) covers the trusted proxy and an ignored untrusted X-Forwarded-For;
|
|
|
|
|
- describes S10: /reset-password (tokens_valid_after only; email_verified_at unchanged) and the admin password set truncate to the second; change-password and bouncer untouched; TestPasswordCutoffSameSecond (Docker) covers iat equal to the cutoff (accepted on jwt.auth, /fetch and /refresh) and one second earlier (refused);
|
|
|
|
|
- ends with: Reply note items 9 and 10 filled. Not tagged, not pushed: ships with the v0.5.0 wave.
|
|
|
|
|
Leave the todo in pending/.
|
|
|
|
|
|
|
|
|
|
Do not commit, do not stage and do not touch STATE.md, state.json or any other file: the orchestrator commits planning docs and handles STATE and g15office.
|
|
|
|
|
</action>
|
|
|
|
|
<verify>
|
|
|
|
|
<automated>cd /media/nvme/dev/golem15/summercms.io/summercms/summercms.go && N=.planning/notes/2026-10-22-external-user-plugin-escalations.md && T=.planning/todos/pending/2026-10-22-user-login-lockout-and-same-second-cutoff.md && P=/media/nvme/dev/golem15/summercms.io/summercms/sm-demo-app/plugins/golem15/user && S9=$(git -C "$P" log -1 --format=%h --fixed-strings --grep='per client address behind trusted proxies' master) && S10=$(git -C "$P" log -1 --format=%h --fixed-strings --grep='stamp tokens_valid_after in whole seconds' master) && test -n "$S9" && test -n "$S10" && grep -qE '^\| 9 \| Lockout keyed on `RemoteAddr` \| Fix \| \[261022-vfz\].*\| sniewiif \| done \(unreleased\) \| `[0-9a-f]{7,}` → v0\.5\.0 \|$' "$N" && grep -qE '^\| 10 \| Same-second token refusal \| Fix \| \[261022-vfz\].*\| sniewiif \| done \(unreleased\) \| `[0-9a-f]{7,}` → v0\.5\.0 \|$' "$N" && grep -qF "\`$S9\` → v0.5.0" "$N" && grep -qF "\`$S10\` → v0.5.0" "$N" && grep -qF "Done locally (\`$S9\`)" "$N" && grep -qF "Done locally (\`$S10\`)" "$N" && grep -qF "Ships in SummerCMS v0.5.0 (sm-user-plugin $S9, tagged with that wave)" "$N" && grep -qF "Ships in SummerCMS v0.5.0 (sm-user-plugin $S10, tagged with that wave)" "$N" && awk '/^Login lockout keyed on RemoteAddr$/{f=1;next} f&&(/^$/||/^```/){f=0} f&&/^TODO/{bad=1} END{exit bad}' "$N" && awk '/^Same-second token refusal$/{f=1;next} f&&(/^$/||/^```/){f=0} f&&/^TODO/{bad=1} END{exit bad}' "$N" && grep -qF 'TODO: ships in SummerCMS v0.5.0 — config key <name>, HttpOnly' "$N" && grep -qF 'TODO: ships in SummerCMS v0.5.0 — config key <name>.' "$N" && grep -qF 'Ships in SummerCMS v0.5.0 (sm-user-plugin 4bf44fc, tagged with that wave)' "$N" && grep -q '^## Outcome' "$T" && awk '/^## Outcome/{o=NR} /^## Invoke/{i=NR} END{exit !(o && i && o<i)}' "$T" && grep -qF "$S9" "$T" && grep -qF "$S10" "$T" && test ! -e .planning/todos/done/2026-10-22-user-login-lockout-and-same-second-cutoff.md && D=$(git diff --name-only -- "$N" "$T") && grep -qxF "$N" <<<"$D" && grep -qxF "$T" <<<"$D" && ST=$(git diff --cached --name-only) && test -z "$ST"</automated>
|
|
|
|
|
</verify>
|
|
|
|
|
<done>Escalation note rows 9 and 10 read done (unreleased) with `S9` → v0.5.0 and `S10` → v0.5.0 and link the quick and the todo. Verdicts 9 and 10 say "Done locally" with their SHAs. The two reply paragraphs carry "Ships in SummerCMS v0.5.0 (sm-user-plugin <sha>, tagged with that wave): ..." lines, and only the TODO lines for items 7 and 8 remain. The todo has an Outcome section before Invoke naming both commits and tests, and it is still in pending/. Both files are modified, unstaged and uncommitted; nothing else in summercms.go was touched.</done>
|
|
|
|
|
</task>
|
|
|
|
|
|
|
|
|
|
</tasks>
|
|
|
|
|
|
|
|
|
|
<threat_model>
|
|
|
|
|
## Trust Boundaries
|
|
|
|
|
|
|
|
|
|
| Boundary | Description |
|
|
|
|
|
|----------|-------------|
|
|
|
|
|
| client or reverse proxy to POST /login | untrusted credentials and an untrusted X-Forwarded-For header; only RemoteAddr is a network fact |
|
|
|
|
|
| session token to jwt.auth, /fetch and /refresh | an untrusted token's iat is compared with the user's tokens_valid_after |
|
|
|
|
|
|
|
|
|
|
## STRIDE Threat Register
|
|
|
|
|
|
|
|
|
|
| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan |
|
|
|
|
|
|-----------|----------|-----------|----------|-------------|-----------------|
|
|
|
|
|
| T-vfz-01 | Denial of service | /login throttle behind a reverse proxy (one client's five failures suspend the account for every visitor) | high | mitigate | Task 1 keys the throttle on surf.ClientIP with http.trusted_proxies; TestLoginThrottleClientIP proves a second client behind the same proxy still signs in while the failing client is suspended. |
|
|
|
|
|
| T-vfz-02 | Spoofing | X-Forwarded-For forged to dodge a lockout or to suspend another client's address | high | mitigate | surf.ClientIP honours X-Forwarded-For only when RemoteAddr is inside http.trusted_proxies and takes the rightmost untrusted hop; TestLoginThrottleClientIP's untrusted subtest proves a changing forged header neither moves the counter nor escapes the suspension. |
|
|
|
|
|
| T-vfz-03 | Elevation of privilege | whole-second cutoff after a password reset (a token issued earlier within the reset's second stays valid) | low | accept | Inherent to whole-second iat; identical to RevokeUserAccess's documented trade-off. The alternative, refusing the user's own same-second login, is the reported bug. Tokens from any earlier second are refused (TestPasswordCutoffSameSecond). |
|
|
|
|
|
| T-vfz-04 | Denial of service | deployments that never set http.trusted_proxies keep keying on the proxy address | medium | transfer | Operator configuration. The README bullet and the reply paragraph tell operators to list their proxy; the default stays RemoteAddr-only, so no deployment starts trusting forged headers. |
|
|
|
|
|
| T-vfz-05 | Tampering | key format change for IPv4-mapped IPv6 and unparseable RemoteAddr (ClientIP unmaps and returns ""), which orphans throttle rows written before the upgrade | low | accept | At most one suspension window (15 minutes by default) is forgotten for such clients at upgrade. Counting resumes on the new key. |
|
|
|
|
|
| T-vfz-SC | Tampering | npm/pip/cargo/Go module installs | low | accept | No dependency is added; go.mod and go.sum are unchanged (both commit gates check it). No package-legitimacy checkpoint is needed. |
|
|
|
|
|
</threat_model>
|
|
|
|
|
|
|
|
|
|
<verification>
|
|
|
|
|
- sm-user-plugin: GOWORK=off go vet ./..., GOWORK=off go test -short -count=1 ./... and GOWORK=off go test -count=1 ./... are green at both fix commits.
|
|
|
|
|
- TestLoginThrottleClientIP was red at 4bf44fc and is green at the first commit. TestPasswordCutoffSameSecond was red at the first commit and is green at the second.
|
|
|
|
|
- The sociallogin and oauthserver suites, run through sm-demo-app's go.work against this checkout, have their result recorded in the SUMMARY.
|
|
|
|
|
- Two local commits on sm-user-plugin master: no trailer, not pushed, not tagged. The sm-demo-app pointer is unchanged; summercms.go modules/bouncer and modules/surf are unmodified.
|
|
|
|
|
- The escalation note and the todo in summercms.go are filled and uncommitted.
|
|
|
|
|
</verification>
|
|
|
|
|
|
|
|
|
|
<success_criteria>
|
|
|
|
|
- Behind a configured trusted proxy, the /login lockout of one client does not lock out other clients of the same account; without trusted proxies, X-Forwarded-For has no effect on the throttle.
|
|
|
|
|
- A login (or any token) issued in the same second as a reset-password or an admin password set is accepted everywhere; earlier seconds are refused.
|
|
|
|
|
- Exported RemoteIP, change-password's cutoff and bouncer's comparison are unchanged.
|
|
|
|
|
- Items 9 and 10 are recorded as done (unreleased) with their SHAs, and the reply is ready apart from items 7 and 8.
|
|
|
|
|
</success_criteria>
|
|
|
|
|
|
|
|
|
|
<output>
|
|
|
|
|
Create `.planning/quick/261022-vfz-user-plugin-login-throttle-via-surf-clie/261022-vfz-SUMMARY.md` in summercms.go when done. Include both plugin commits' short SHAs and the base, both RED failure lines, the suite results, the consumer cross-check result, the logged same-second observation from TestPasswordCutoffSameSecond, and a confirmation that nothing was pushed, tagged or pointer-bumped and that the note and todo are left uncommitted. Do not commit the SUMMARY; the orchestrator commits planning docs.
|
|
|
|
|
</output>
|
Jakub Zych