From cdd647243c0af86a22673c72d15c6af3232c6891 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Wed, 21 Oct 2026 02:15:06 +0200 Subject: [PATCH] docs(19.5-04): complete unit tests, PHP map, security review and v0.1.1 plan --- .../19.5-04-SUMMARY.md | 370 ++++++++++++++++++ 1 file changed, 370 insertions(+) create mode 100644 .planning/phases/19.5-social-login-in-sm-user-plugin/19.5-04-SUMMARY.md diff --git a/.planning/phases/19.5-social-login-in-sm-user-plugin/19.5-04-SUMMARY.md b/.planning/phases/19.5-social-login-in-sm-user-plugin/19.5-04-SUMMARY.md new file mode 100644 index 00000000..e26b8640 --- /dev/null +++ b/.planning/phases/19.5-social-login-in-sm-user-plugin/19.5-04-SUMMARY.md @@ -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*