golem15/summercms
4
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

docs(19.5-04): complete unit tests, PHP map, security review and v0.1.1 plan

Browse Source
This commit is contained in:
Jakub Zych
2026-10-21 02:15:06 +02:00
parent f38713f540
commit cdd647243c
1 changed files with 370 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

370
.planning/phases/19.5-social-login-in-sm-user-plugin/19.5-04-SUMMARY.md Normal file
View File

@@ -0,0 +1,370 @@
---
phase: 19.5-social-login-in-sm-user-plugin
plan: 04
subsystem: auth
tags: [sociallogin, oauth, unit-tests, php-map, security-review, gate, release, pre-account-takeover]
requires:
- phase: 19.5-01
provides: "cooler.Cache.Pull, sm-user-plugin social sign-in hooks (v0.1.4)"
- phase: 19.5-02
provides: "sm-sociallogin-plugin v0.1.0, summercms v0.3.1"
- phase: 19.5-03
provides: "sm-demo-app and sm-bm-app mounting sociallogin v0.1.0, fonoteka.go at sm-user-plugin v0.1.4"
provides:
- "scripts/check-phase19.5.sh: fail-closed gate across six repositories (go, userplugin, sociallogin, demo, bm, fonoteka, docker, docs, self-test, all)"
- "Unit tests: cooler Pull (4), sm-user-plugin (6 + 1 PHP-named subtest), sm-sociallogin-plugin (26 decision tests + TestPHPMethodMap + 5 TestPHP* ports)"
- "testdata/PHP_METHOD_MAP.md: 38 PHP social-auth methods, 17 ported, 18 adapted, 3 not-applicable"
- "19.5-SECURITY-REVIEW.md: every T-19.5 threat pinned or accepted; T-19.5-38 found and fixed"
- "sm-sociallogin-plugin v0.1.1 (three fixes), pinned in sm-demo-app and sm-bm-app"
affects: [sm-sociallogin-plugin, sm-user-plugin, sm-demo-app, sm-bm-app, /msd-verify-work]
estimate:
tokens: 320000
tasks: 7
actuals:
tokens: 78000
tasks: 7
commits: 9
plan_head_before: 877bf0a425d952df8b323a56d1a0b284474ff97e
plan_head_after: f38713f
# commits counts this plan's summercms.go commits before this SUMMARY: git rev-list --count 877bf0a..f38713f = 13,
# of which 4 belong to another session (bc5a9db, 63a85a8, 09fd4d0, cdddaa3, phase 19.8 docs), so 9 are this plan's.
# Other repositories (measured from their ledgers):
# sm-sociallogin-plugin: git rev-list --count adfcfbf..d7ee28d = 6 (pushed, tag v0.1.1)
# sm-user-plugin: git rev-list --count c43ba87..c7635c5 = 2 (pushed, no tag)
# sm-demo-app: 61a00e9 (pushed); sm-bm-app: 99af8c2 (pushed with the approved earlier commits)
# tokens: chars/4 over the test files, the gate script, the three fixes and the planning docs written by this plan.
tech-stack:
added: []
patterns:
- "Fail-closed phase gate: go test -json detector (fail, skip, zero, no-tests, missing name), vitest detector, NOT RUN docker rule, --self-test on planted output"
- "Forced races: explicit transactions with row locks in both orders; start barriers with N goroutines; mutation checks on the mitigation"
- "Built-in goth wrappers under test through a registry published before the plugin registers (bootSocialWith) with a fake http.RoundTripper"
- "Transaction-scoped advisory locks (pg_advisory_xact_lock) to serialise concurrent account creations of one identity and email"
key-files:
created:
- scripts/check-phase19.5.sh
- modules/cooler/pull_test.go
- .planning/phases/19.5-social-login-in-sm-user-plugin/19.5-SECURITY-REVIEW.md
- .planning/todos/pending/oauth-password-bootstrap-change-password.md
- ../fonoteka.go/plugins/golem15/user/oauth_providers_test.go
- ../fonoteka.go/plugins/golem15/user/oauth_identities_rule_test.go
- ../fonoteka.go/plugins/golem15/user/social_hooks_test.go
- ../sm-sociallogin-plugin/unit_helpers_test.go
- ../sm-sociallogin-plugin/registry_test.go
- ../sm-sociallogin-plugin/statecookie_test.go
- ../sm-sociallogin-plugin/normalize_test.go
- ../sm-sociallogin-plugin/codes_test.go
- ../sm-sociallogin-plugin/routes_test.go
- ../sm-sociallogin-plugin/matching_test.go
- ../sm-sociallogin-plugin/registration_test.go
- ../sm-sociallogin-plugin/link_test.go
- ../sm-sociallogin-plugin/complete_test.go
- ../sm-sociallogin-plugin/register_complete_test.go
- ../sm-sociallogin-plugin/settings_test.go
- ../sm-sociallogin-plugin/providers_goth_test.go
- ../sm-sociallogin-plugin/php_port_test.go
- ../sm-sociallogin-plugin/php_method_map_test.go
- ../sm-sociallogin-plugin/testdata/php_methods.txt
- ../sm-sociallogin-plugin/testdata/PHP_METHOD_MAP.md
modified:
- modules/cooler/docker_test.go
- .planning/phases/19.5-social-login-in-sm-user-plugin/19.5-VALIDATION.md
- ../fonoteka.go/plugins/golem15/user/profile_test.go
- ../sm-sociallogin-plugin/classes/flow.go
- ../sm-sociallogin-plugin/classes/registration.go
- ../sm-sociallogin-plugin/models/settings.go
- ../sm-sociallogin-plugin/harness_test.go
- ../sm-sociallogin-plugin/README.md
- ../sm-demo-app/plugins/golem15/sociallogin (pointer), go.mod, go.sum
- ../sm-bm-app/plugins/golem15/sociallogin (pointer)
key-decisions:
- "Task 6 reply: release sm-sociallogin-plugin v0.1.1; push sm-user-plugin test commits; push sm-bm-app including the earlier approved commits; fix T-19.5-38 with option (b)"
- "T-19.5-38 option (b): a verified email auto-links only onto an account with a self-set password; a password-less account goes pending and its completion answers email_exists_link_account (deliberate divergence from PHP)"
- "Gate scripts and .planning docs count as non-production for the Post-release fixes table (no release needed)"
- "Concurrent account creations are serialised with transaction-scoped advisory locks on the provider identity and the email"
- "The relaxed change-password bootstrap of the PHP plugin is not ported (3 not-applicable map rows); tracked as a todo"
patterns-established:
- "A phase's last plan ships a fail-closed gate script plus a PHP method map with a checker that fails on missing, extra or nonexistent tests"
requirements-completed: [SC-4, D-03, D-04, D-06, D-07, D-08, D-09, D-10, D-11, D-12, D-13, D-14, D-15, D-16, D-17, D-19, D-20, D-21, D-23, D-25]
coverage:
- id: D1
description: "Fail-closed gate across six repositories, detectors proven on planted output"
requirement: "SC-4"
verification:
- kind: other
ref: "scripts/check-phase19.5.sh --all (exit 0, docker stage run) and --self-test"
status: pass
human_judgment: false
- id: D2
description: "cooler.Cache.Pull single winner, expiry and prefix isolation on memory, miniredis and redis:7-alpine"
requirement: "D-08"
verification:
- kind: unit
ref: "modules/cooler/pull_test.go#TestPullConcurrent, TestPullExpired, TestPullPrefixIsolation; docker_test.go#TestPullDocker"
status: pass
human_judgment: false
- id: D3
description: "sm-user-plugin provider source, unlink rule and race, sign-in helpers"
requirement: "D-13"
verification:
- kind: integration
ref: "sm-user-plugin oauth_providers_test.go, oauth_identities_rule_test.go, social_hooks_test.go (Postgres)"
status: pass
human_judgment: false
- id: D4
description: "One unit test per sociallogin decision, incl. nOAuth, state cookie, concurrent completion, pre-account takeover"
requirement: "D-10"
verification:
- kind: integration
ref: "sm-sociallogin-plugin gate --sociallogin (9 tests) and --docker (28 tests)"
status: pass
human_judgment: false
- id: D5
description: "PHP social-auth suite mapped case by case (38 methods) with a checker"
requirement: "D-25"
verification:
- kind: unit
ref: "sm-sociallogin-plugin php_method_map_test.go#TestPHPMethodMap (mutation-checked) and the five TestPHP* ports"
status: pass
human_judgment: false
- id: D6
description: "Security review: every T-19.5 threat pinned by a test or accepted; T-19.5-38 fixed"
requirement: "D-11"
verification:
- kind: other
ref: ".planning/phases/19.5-social-login-in-sm-user-plugin/19.5-SECURITY-REVIEW.md; TestPreAccountTakeover"
status: pass
human_judgment: true
rationale: "Review completeness and the T-19.5-38 product decision are judgments; the verifier should read the register"
- id: D7
description: "Post-release fixes released as v0.1.1 and contained in every application pin"
requirement: "D-21"
verification:
- kind: other
ref: "git ls-remote --exit-code --tags origin v0.1.1; merge-base --is-ancestor of 21f3fe5, c99f5f9, d7ee28d against the sociallogin gitlinks of sm-demo-app and sm-bm-app (all exit 0); gate --demo and --bm at the new pointers"
status: pass
human_judgment: false
- id: D8
description: "Live sign-in with real provider credentials in the demo and bm"
requirement: "D-21"
human_judgment: true
rationale: "Needs real provider registrations; manual row in VALIDATION.md"
duration: 1h42m
completed: 2026-10-21
status: complete
---
# Phase 19.5 Plan 04: Unit tests, PHP map, security review and v0.1.1 Summary
**A fail-closed six-repository gate, unit tests for every social sign-in decision (cooler Pull, sm-user-plugin hooks, 26 plugin tests on Postgres and fake providers), the 38-method PHP social-auth map with a checker, and a security review that found three defects in sm-sociallogin-plugin (500s on concurrent completions and on every settings save, and a pre-account takeover through unverified-email accounts), fixed and released as v0.1.1 into sm-demo-app and sm-bm-app**
## Performance
- **Duration:** about 1h42m (2026-10-20T22:31:38Z to 2026-10-21T00:14Z), including the Task 6 checkpoint wait
- **Tasks:** 7 of 7 (Task 6 the user's go-ahead, Task 7 the release)
- **Files:** about 40 created or modified across six repositories
## Accomplishments
- `scripts/check-phase19.5.sh --all` exits 0 with every stage PASS, including docker (no NOT RUN).
- Every decision of the validation map has a unit test, and the gate runs it by name.
- The PHP social-auth suite is mapped: 17 ported, 18 adapted, 3 not-applicable (the relaxed change-password bootstrap, now a todo).
- The security review pins every threat T-19.5-01..37 and T-19.5-SC, or accepts it with a reason; it lists every log call.
- Three production defects found by the new tests and the review were fixed, released as sm-sociallogin-plugin v0.1.1 and pinned in both applications.
## Task Commits
summercms.go:
1. Task 1: fail-closed phase gate. `b4c0e0d`
2. Task 2: cooler Pull tests. `671d388`
3. Task 2: gate update. `6b8f060`
4. Task 3: gate update. `18598c1`
5. Task 4: password-bootstrap todo. `52f17e4`
6. Task 4: gate the PHP map. `2c2ff81`
7. Task 5: security review and validation map. `e830298`
8. Post-checkpoint: gate `TestPreAccountTakeover`. `1d5dc36`
9. Post-checkpoint: close T-19.5-38 in the review. `f38713f`
sm-user-plugin (`fonoteka.go/plugins/golem15/user`), pushed `c43ba87..c7635c5`, no tag:
- Task 2: provider source, unlink rule and race, sign-in helpers. `d6d8f11`
- Task 4: `TestChangePassword/real_password_account_still_requires_current_password`. `c7635c5`
sm-sociallogin-plugin, pushed `adfcfbf..d7ee28d`, tag **v0.1.1** (tag object `dd1c1de`, on `d7ee28d`):
- Task 3: fix concurrent pending completions (410, not 500). `21f3fe5`
- Task 3: fix the admin settings save. `c99f5f9`
- Task 3: unit tests for every decision. `f18830d`
- Task 4: PHP map and ports. `d9a72a7`
- Task 5: key domain-separation pins. `b7fa407`
- Task 7 (after the Task 6 reply): fix the pre-account takeover (T-19.5-38). `d7ee28d`
Applications:
- sm-demo-app: `61a00e9` chore: bump sm-sociallogin-plugin to v0.1.1 (gitlink and go.mod require v0.1.1), pushed `d5a98d9..61a00e9`.
- sm-bm-app: `99af8c2` chore: bump sm-sociallogin-plugin to v0.1.1, pushed `9d5a371..99af8c2`. The push carried, as approved, another session's `3a97157` and `1d446cc` and plan 03's `bb800a5`, `117ef8f`, `e76f8bc`, `0374c3f`. Nothing else was unpushed, and every submodule pointer in the push was already on its remote.
- fonoteka.go: not touched (user pin unchanged at c43ba87).
## Gate output
`scripts/check-phase19.5.sh --all`, last run after the T-19.5-38 fix: exit 0.
- self-test: PASS
- go: PASS (4 cooler tests)
- userplugin: PASS (2 tests; additive check against 4c5d3b8, 10 declarations added, no migration changed)
- sociallogin: PASS (9 tests; no gorilla/sessions or gopkg.in/yaml.v3; goth only in `classes/providers_goth.go`)
- demo: PASS (3)
- bm: PASS (Go 1 + 2; vitest 211 passed; typecheck)
- fonoteka: PASS (vet, short suite, sociallogin not mounted)
- docs: PASS
- docker: PASS (`TestPullDocker`; 10 user-plugin tests; 28 sociallogin tests)
After the release: `--demo`, `--bm` and `--sociallogin` passed again at the new pointers. `--fonoteka` was not rerun: nothing it tests changed since the `--all` run.
## Post-release fixes
| Repo | Commit | Subject | Found by | Pins that lacked it | Outcome |
|---|---|---|---|---|---|
| sm-sociallogin-plugin | `21f3fe5` | fix: concurrent pending completions answer 410, not 500 | `TestRegisterComplete/concurrent_submit_one_winner` (T-19.5-12) | sm-demo-app and sm-bm-app `plugins/golem15/sociallogin` (adfcfbf) | released in v0.1.1; both pins at d7ee28d; `merge-base --is-ancestor` exit 0 |
| sm-sociallogin-plugin | `c99f5f9` | fix: the admin settings form saves (tenant checked without a regex rule) | `TestSettings/admin_json_hides_secrets` (T-19.5-16) | same two pins | released in v0.1.1; exit 0 for both |
| sm-sociallogin-plugin | `d7ee28d` | fix: a verified sign-in never links onto a password-less account | security review finding T-19.5-38, pinned by `TestPreAccountTakeover` | same two pins | released in v0.1.1; exit 0 for both |
sm-user-plugin and summercms.go had no production change (test, gate and planning commits only). So there was no user-plugin or framework tag, no changelog post for the framework, and no change to the plugin's requires (still summercms v0.3.1, sm-user-plugin v0.1.4).
## Task 6 reply
> release sm-sociallogin-plugin v0.1.1, approve both extra pushes, fix T-19.5-38 with option (b)
As relayed by the coordinator from the user's answers in the main session. Interpreted as:
- tag v0.1.1 after fixing T-19.5-38;
- push sm-user-plugin `d6d8f11` and `c7635c5` without a tag;
- push sm-bm-app including the earlier approved commits;
- no changelog post from this executor (the main session handles it).
## Tags and final pins
| Repository | Tag | Commit |
|---|---|---|
| sm-sociallogin-plugin | v0.1.1 (new, pushed) | d7ee28d |
| sm-user-plugin | v0.1.4 (unchanged; master pushed to c7635c5) | c43ba87 |
| summercms.go | v0.3.1 (unchanged) | |
| Application | sociallogin | user |
|---|---|---|
| sm-demo-app | d7ee28d (v0.1.1; go.mod v0.1.1) | c43ba87 (v0.1.4) |
| sm-bm-app | d7ee28d (v0.1.1; go.mod replace to the submodule) | c43ba87 (v0.1.4) |
| fonoteka.go | not mounted | c43ba87 (v0.1.4) |
v0.1.1 tag message (user-facing):
```
sm-sociallogin-plugin v0.1.1: security and reliability fixes
- a verified sign-in (Google, GitHub) no longer links onto an existing
account that has no self-set password: an account created from an email
the provider did not verify (Facebook, Microsoft) goes to the pending
screen instead, and its owner links further providers from the settings
or proves the address with a password reset (closes a pre-account
takeover)
- the admin settings page saves again: the Microsoft tenant is checked by
the model instead of an unsupported validation rule, and a malformed
tenant is a validation error on the field
- two simultaneous submissions of one pending registration answer one 200
and one 410 instead of a 500
- unit tests for every decision and the PHP social-auth test map
- still requires summercms v0.3.1 and sm-user-plugin v0.1.4
```
## PHP map
38 methods: 17 ported, 18 adapted, 3 not-applicable. 26 rows point to sm-sociallogin-plugin tests and 9 to sm-user-plugin tests.
- The three not-applicable rows are PHP's relaxed change-password bootstrap; the todo `oauth-password-bootstrap-change-password.md` tracks it.
- `test_verified_provider_email_still_auto_links` is adapted, marking the deliberate T-19.5-38 divergence.
## Decisions Made
- T-19.5-38 option (b): `ownerVerifiedEmail` = the matched account has a self-set password. A password reset through the mailed code sets it, so the owner can prove the address. No sm-user-plugin change was needed; the fix only reads the existing `HasSelfSetPassword` field.
- The refused auto-link reuses the existing shapes: pending `reason=unverified_match` at the callback, 422 `email_exists_link_account` at the completion.
## Deviations from Plan
### Auto-fixed Issues
**1. [Rule 1 - Bug] Concurrent pending completions answered 500**
- **Found during:** Task 3 (`TestRegisterComplete/concurrent_submit_one_winner`: 15 × 500 instead of 410)
- **Fix:** advisory locks on the identity and the email during account creation; a completion that loses the race finds the identity and returns `ErrPendingConsumed`
- **Files modified:** `classes/flow.go`, `classes/registration.go`
- **Commit:** `21f3fe5`
**2. [Rule 1 - Bug] Every admin settings save answered 500**
- **Found during:** Task 3 (`TestSettings`). lagoon's model validation has no `regex:` rule.
- **Fix:** tenant format checked in `Settings.BeforeValidate` (422); README updated
- **Files modified:** `models/settings.go`, `README.md`
- **Commit:** `c99f5f9`
**3. [Rule 4 - User decision] Pre-account takeover (T-19.5-38), found by the security review**
- **Process:** raised at the Task 6 checkpoint; the user chose option (b)
- **Fix:** a verified email auto-links only onto an account with a self-set password
- **Test:** `TestPreAccountTakeover` (mutation-checked)
- **Docs:** README matching table and security notes; PHP map row adapted
- **Commit:** `d7ee28d`
### Other deviations
4. **[Process] One context for Tasks 1–5** instead of the planned hand-off after Task 3 (the budget allowed). At the Task 6 checkpoint, the Post-release fixes table went into the checkpoint return instead of a draft SUMMARY, as the coordinator instructed.
5. **[Rule 3] Harness changes in sm-sociallogin-plugin:**
- the harness trusts the loopback proxy, so tests choose the client address with `X-Forwarded-For` (deterministic throttle tests);
- `bootSocialWith` publishes a test registry before the plugins register;
- added `unit_helpers_test.go`.
6. **[Rule 2] User-plugin Postgres tests skip under `-short`** (CLAUDE.md rule); the gate runs them by name in the docker stage.
7. **[Scope] Non-production commits not counted as post-release fixes:** gate scripts and `.planning` docs, though the plan's literal rule would list them.
8. **[Context] Normaliser difference from PHP:** a percent-encoded callback path whose decoded form equals the callback path is accepted and rebuilt to the canonical path. Safe, since the output is canonical; PHP refuses it. Pinned by `TestNormalizeFrontendCallback/encoded_path_rebuilt`.
9. **[Context] `TestErrorMessages`** lives in `settings_test.go` (no `messages_test.go` in the plan's file list).
10. **[Context] Pre-existing fonoteka failure:** `TestPhase09SecurityRoutes` (deferred-items.md) is outside the gate. The fonoteka stage runs `-short`, and the docker stage runs only this phase's named tests, so it never blocked a stage.
11. **[Context] summercms.go push:** summercms.go was not pushed (not in the reply). Its unpushed range mixes this plan's commits with another session's 19.8 commits.
12. **[Context] No g15office:** no g15office task for the new todo and no milestone comment (no tool in this session; the main session posts it).
**Total deviations:** 12 (2 bug fixes, 1 user-decided security fix, 9 process or context notes). **Impact:** three production fixes released as v0.1.1; contracts otherwise unchanged.
## Issues Encountered
None open. T-19.5-38 is closed.
## Known Stubs
None.
## Threat Flags
None. No new network surface beyond the plan's threat model; the fixes narrow existing paths.
## User Setup Required
None new (see 19.5-USER-SETUP.md for live provider credentials).
## Next Phase Readiness
- Phase complete; ready for `/msd-verify-work 19.5`.
- The manual row in VALIDATION.md (live sign-in with real providers) remains.
- The main session may post the v0.1.1 notes and the g15office comment.
## Self-Check: PASSED
- Files exist: `scripts/check-phase19.5.sh`, `19.5-SECURITY-REVIEW.md`, `19.5-VALIDATION.md`, the todo, `sm-sociallogin-plugin/testdata/PHP_METHOD_MAP.md` and `php_methods.txt`, `php_method_map_test.go`, `php_port_test.go`, the user-plugin test files.
- Commits exist in their repositories:
- summercms.go: `b4c0e0d 671d388 6b8f060 18598c1 52f17e4 2c2ff81 e830298 1d5dc36 f38713f`
- sm-user-plugin: `d6d8f11 c7635c5` (on origin)
- sm-sociallogin-plugin: `21f3fe5 c99f5f9 f18830d d9a72a7 b7fa407 d7ee28d` (on origin)
- sm-demo-app: `61a00e9`; sm-bm-app: `99af8c2` (both on origin)
- `git ls-remote --exit-code --tags origin v0.1.1` exits 0.
- `origin/master..master` is empty in sm-sociallogin-plugin, sm-user-plugin, sm-demo-app and sm-bm-app.
---
*Phase: 19.5-social-login-in-sm-user-plugin*
*Completed: 2026-10-21*