docs(19.5-05): hand-off draft after Task 3 (email proof in user and sociallogin)
This commit is contained in:
Jakub Zych
@@ -0,0 +1,165 @@
|
||||
---
|
||||
phase: 19.5-social-login-in-sm-user-plugin
|
||||
plan: 05
|
||||
subsystem: auth
|
||||
tags: [sociallogin, sm-user-plugin, email-proof, pre-account-takeover, postgres, gormigrate, gate]
|
||||
status: draft
|
||||
requires:
|
||||
- phase: 19.5-04
|
||||
provides: "sm-user-plugin v0.1.4 social sign-in hooks, sm-sociallogin-plugin v0.1.1 matching (ownerVerifiedEmail), scripts/check-phase19.5.sh"
|
||||
provides:
|
||||
- "users.email_verified_at (migration 202610210001_add_users_email_verified_at), written only by VerifyActivationCode and ResetPassword, cleared on an email change"
|
||||
- "sociallogin ownerVerifiedEmail reads EmailVerifiedAt (D-26)"
|
||||
- "TestOwnerVerifiedEmail (8 subtests), TestEmailVerifiedAt, TestAdminEmailVerifiedAt, TestEmailVerifiedAtMigration"
|
||||
- "gate: USER_ADDED_MIGRATIONS, refused_migration_changes, migration_writes_rows, DOCKER_USER_UPDATES_TESTS"
|
||||
affects: [19.5-05 Task 4 (bm view test), 19.5-05 Task 5 (security review, validation map), 19.5-05 Task 7 (release)]
|
||||
actuals:
|
||||
tokens: 18621 # chars/4 over the Task 1-3 diffs (sm-user-plugin, sm-sociallogin-plugin, gate script)
|
||||
tasks: 3 # of 7; hand-off after Task 3
|
||||
commits: 2 # summercms.go only (7170c5b, 30e0cd9); rev-list from the ledger counts 4 because two 19.8-03 commits of another session landed in between
|
||||
plan_head_before: 33460607db95276e8180487a8ece7e4f0070b1e7
|
||||
plan_head_after: 30e0cd9
|
||||
tech-stack:
|
||||
added: []
|
||||
patterns:
|
||||
- "Proof of address as a nullable timestamp column, stamped in the same write as the mailed-code action that proves it"
|
||||
- "Gate migration allow-list: git diff --name-status filtered through refused_migration_changes, plus a comment-blind SQL write scan"
|
||||
key-files:
|
||||
created:
|
||||
- ../fonoteka.go/plugins/golem15/user/updates/202610210001_add_users_email_verified_at.go
|
||||
- ../fonoteka.go/plugins/golem15/user/updates/email_verified_at_test.go
|
||||
- ../fonoteka.go/plugins/golem15/user/email_verified_test.go
|
||||
- ../sm-sociallogin-plugin/owner_verified_test.go
|
||||
modified:
|
||||
- ../fonoteka.go/plugins/golem15/user/models/user.go
|
||||
- ../fonoteka.go/plugins/golem15/user/classes/codes.go
|
||||
- ../fonoteka.go/plugins/golem15/user/controllers/api_controller.go
|
||||
- ../fonoteka.go/plugins/golem15/user/controllers/users_admin_controller.go
|
||||
- ../fonoteka.go/plugins/golem15/user/updates/organisations_test.go
|
||||
- ../fonoteka.go/plugins/golem15/user/README.md
|
||||
- ../sm-sociallogin-plugin/classes/flow.go
|
||||
- ../sm-sociallogin-plugin/harness_test.go
|
||||
- ../sm-sociallogin-plugin/matching_test.go
|
||||
- ../sm-sociallogin-plugin/testdata/PHP_METHOD_MAP.md
|
||||
- ../sm-sociallogin-plugin/README.md
|
||||
- scripts/check-phase19.5.sh
|
||||
- .planning/phases/19.5-social-login-in-sm-user-plugin/deferred-items.md
|
||||
key-decisions:
|
||||
- "Proof writers are exactly VerifyActivationCode (activate, activate-by-code, invitation link) and ResetPassword's password write; RegisterUser, admin create, ActivateUsers, ChangePassword and every sociallogin path write nothing"
|
||||
- "An email change through /update or the admin form clears email_verified_at; a case-only change (strings.EqualFold) keeps it"
|
||||
- "Harness e.user is a proven account so plan 02-04 link cases keep testing the link path; e.unprovenUser models accounts without proof"
|
||||
requirements-completed: [] # D-26, D-27, D-28, SC-3 are marked by the final context (Task 7), not this hand-off
|
||||
duration: 29min
|
||||
completed: 2026-10-21
|
||||
---
|
||||
|
||||
# Phase 19.5 Plan 05: Proof of Address for Social Sign-in Linking Summary
|
||||
|
||||
Status: draft (hand-off after Task 3)
|
||||
|
||||
**users.email_verified_at (stamped only by the mailed activation code and the completed reset-code password reset, cleared on an email change) is now the owner-side condition for auto-linking a verified provider, so an auto-mode `/register` account goes pending instead of handing its sign-in to a squatter**
|
||||
|
||||
This draft covers Tasks 1-3 of 7. A fresh agent runs Tasks 4 (bm view test) and 5 (security review, validation map, `--all`, release table) and stops at the Task 6 checkpoint; Task 7 finalizes this file.
|
||||
|
||||
## Performance
|
||||
|
||||
- **Duration:** 29 min (this context)
|
||||
- **Started:** 2026-10-21T11:37:14Z
|
||||
- **Hand-off:** 2026-10-21T12:06:29Z
|
||||
- **Tasks:** 3 of 7
|
||||
- **Files modified:** 17 across three repositories (plus deferred-items.md)
|
||||
|
||||
## Accomplishments
|
||||
|
||||
- Migration `202610210001_add_users_email_verified_at`: `ALTER TABLE users ADD COLUMN IF NOT EXISTS email_verified_at TIMESTAMPTZ NULL`, no row written, no-op rollback; `User.EmailVerifiedAt` with `json:"-"`, not fillable.
|
||||
- `classes.VerifyActivationCode` stamps the proof in the same `UpdateColumns` write that activates; `controllers.ResetPassword` stamps it in the `Updates` that stores the new password.
|
||||
- `controllers.Update` and `usersAdminController.FormBeforeUpdate` clear it when the email changes (case-only change keeps it). `storedEmail` is an unexported helper now shared with `guardCredentials`.
|
||||
- sociallogin `ownerVerifiedEmail(u)` is `u != nil && u.EmailVerifiedAt != nil`; `classes/flow.go` no longer reads `HasSelfSetPassword`.
|
||||
- Gate: the userplugin stage accepts exactly the one added migration and refuses any other added, modified, renamed or deleted non-test file under `updates/`, and a migration that writes or drops data; the new tests are required by name.
|
||||
|
||||
## Commits per repository
|
||||
|
||||
**sm-user-plugin** (`../fonoteka.go/plugins/golem15/user`, base c7635c5 = v0.1.4+1, nothing pushed):
|
||||
|
||||
| Task | Commit | Message |
|
||||
|------|--------|---------|
|
||||
| 1 | `add1f7a` | feat: users.email_verified_at records a proved address (mail activation, reset code) |
|
||||
| 2 | `a99cb46` | fix: an email change drops the address proof |
|
||||
| 2 | `35cf166` | test: email_verified_at writers, non-writers and the migration |
|
||||
|
||||
**sm-sociallogin-plugin** (`../sm-sociallogin-plugin`, base d7ee28d = v0.1.1, nothing pushed; go.mod still requires sm-user-plugin v0.1.4 until Task 7, the committed go.work points at the workspace checkout):
|
||||
|
||||
| Task | Commit | Message |
|
||||
|------|--------|---------|
|
||||
| 1 | `3bb5650` | fix: a verified sign-in links only onto an account that proved its address |
|
||||
| 3 | `962190b` | test: owner proof table for verified sign-ins |
|
||||
| 3 | `c4ebf05` | docs: proof of address in the matching table and the PHP map |
|
||||
|
||||
**summercms.go** (gate script; base 3346060):
|
||||
|
||||
| Task | Commit | Message |
|
||||
|------|--------|---------|
|
||||
| 1 | `7170c5b` | test(19.5): gate the email_verified_at migration and TestOwnerVerifiedEmail |
|
||||
| 2 | `30e0cd9` | test(19.5): gate the user-plugin proof tests |
|
||||
|
||||
Two foreign commits of another session (`2057643`, `4895b25`, both `fix(19.8-03)`) landed between them on summercms.go master; they are not this plan's.
|
||||
|
||||
**sm-bm-app, sm-demo-app, fonoteka.go:** no commit in this context. fonoteka.go's worktree shows `plugins/golem15/user` modified (the checkout is now at 35cf166); it is not staged until Task 7.
|
||||
|
||||
## Tests added (all pass)
|
||||
|
||||
- **sm-sociallogin-plugin** `TestOwnerVerifiedEmail` (owner_verified_test.go): `register_auto_then_google_pending`, `mail_activated_by_code_links`, `reset_code_password_links` (Task 1); `legacy_account_pending`, `register_action_pending`, `admin_activation_not_proof`, `email_change_drops_proof`, `unverified_provider_never_links_proven_account` (Task 3). Helpers on `*socialEnv`: `ownerEnv`, `postUserAPI`, `registerWithPassword`, `userRow`, `activateByMail`, `resetByMail`, `googleSignIn`, `assertPendingNoLink`, `assertLinkedAndSignedIn`. Harness: `e.user` (proven), `e.unprovenUser`.
|
||||
- **sm-sociallogin-plugin** `TestPreAccountTakeover`: setup also requires `EmailVerifiedAt == nil`; `after_mailbox_proof_links` now drives `/forgot-password` and `/reset-password` through `resetByMail`.
|
||||
- **sm-user-plugin** `TestEmailVerifiedAt` (email_verified_test.go): `register_auto_no_proof`, `register_admin_mode_no_proof`, `register_ignores_submitted_proof`, `activate_by_code_stamps`, `activate_stamps`, `activate_wrong_code_no_proof`, `reset_password_stamps`, `reset_wrong_code_no_proof`, `change_password_no_proof`, `update_email_change_clears`, `update_same_email_keeps`, `update_ignores_submitted_proof`.
|
||||
- **sm-user-plugin** `TestAdminEmailVerifiedAt`: `admin_create_no_proof`, `admin_activate_action_no_proof`, `admin_invite_link_stamps`, `admin_email_change_clears`, `admin_save_keeps_proof`, `admin_json_hides_proof`.
|
||||
- **sm-user-plugin** `TestEmailVerifiedAtMigration` (updates/email_verified_at_test.go): `column_shape`, `no_backfill`, `existing_column_kept`, `rollback_keeps_column`, `history_once`. `TestCreateOrganisationsMigration` expects the new ID last and rolls back 12 steps.
|
||||
|
||||
RED evidence: with the old `ownerVerifiedEmail` (self-set password), `TestOwnerVerifiedEmail/register_auto_then_google_pending` failed (the redirect carried a `code`, i.e. the auto-mode account was linked). With the Task 2 controllers reverted to HEAD, exactly `TestEmailVerifiedAt/update_email_change_clears` and `TestAdminEmailVerifiedAt/admin_email_change_clears` failed.
|
||||
|
||||
## Gate stages (scripts/check-phase19.5.sh) at hand-off
|
||||
|
||||
| Stage | State | Notes |
|
||||
|-------|-------|-------|
|
||||
| --self-test | PASS | new migration-check cases (allowed `A`, refused `M` 00_base.go, other `A`, `D`, `R100`; planted row update refused, word in a `//` comment accepted) |
|
||||
| --userplugin | PASS | "13 declarations added; 1 allowed migration(s) added, column only" |
|
||||
| --sociallogin | PASS | includes TestPHPMethodMap on the edited map |
|
||||
| --fonoteka | PASS | go vet and go test -short of the workspace at the changed user plugin |
|
||||
| --docker | PASS | cooler 1, user root 12, user updates 2, sociallogin 29 top-level tests; run with `TESTCONTAINERS_HOST_OVERRIDE=127.0.0.1` (see Issues) |
|
||||
| --go, --demo, --bm, --docs | NOT RUN | not touched by Tasks 1-3; Task 5 runs `PHASE195_REQUIRE_DOCKER=1 --all` |
|
||||
|
||||
## Decisions Made
|
||||
|
||||
Followed the plan's decisions. Within them:
|
||||
- `storedEmail` was extracted from `guardCredentials` so `FormBeforeUpdate` compares the stored and submitted email without a second copy of the query; `guardCredentials`' behaviour is unchanged.
|
||||
- `TestOwnerVerifiedEmail` boots its own environment with `gothFakeRegistry` (helper `ownerEnv`) so the Google and Facebook wrappers run on fake APIs, as `TestPHPSocialAuthGate` does.
|
||||
|
||||
## Deviations from Plan
|
||||
|
||||
None in production code or tests - Tasks 1-3 executed as written.
|
||||
|
||||
Environment-only adjustments (no repository change):
|
||||
- Docker-backed runs used `TESTCONTAINERS_HOST_OVERRIDE=127.0.0.1` (see Issues Encountered).
|
||||
- `change_password_no_proof` sets `has_self_set_password` before calling change-password, as `TestChangePassword` does (`insertUser` creates the row with the flag false, which change-password answers with an opaque 500).
|
||||
|
||||
## Issues Encountered
|
||||
|
||||
- **Docker host IPv6 loopback.** Published container ports answer on 127.0.0.1 but time out on `[::1]`; pgx resolves `localhost` to `[::1]` first, so every testcontainers suite reported "postgres unavailable" (inside and outside the sandbox). Setting `TESTCONTAINERS_HOST_OVERRIDE=127.0.0.1` fixed it; Task 5's `--all` needs the same. Logged in deferred-items.md.
|
||||
- **Pre-existing:** sm-user-plugin `updates/postgres_test.go` `testShort` scans `os.Args`, so `go test -short` still starts Postgres for the updates package. Out of scope, logged in deferred-items.md.
|
||||
|
||||
## Hand-off for Tasks 4 and 5
|
||||
|
||||
- Task 4 starts from sm-bm-app (no commit yet) and the gate's `detect_vitest`/`run_bm`.
|
||||
- Task 5 records T-19.5-39..46, CR-02 open, VALIDATION rows 19.5-05-01..07, runs `PHASE195_REQUIRE_DOCKER=1 TESTCONTAINERS_HOST_OVERRIDE=127.0.0.1 bash scripts/check-phase19.5.sh --all`, and adds the `## Release table` here.
|
||||
- No tag exists on either plugin's master; nothing was pushed.
|
||||
|
||||
## User Setup Required
|
||||
|
||||
None.
|
||||
|
||||
---
|
||||
*Phase: 19.5-social-login-in-sm-user-plugin*
|
||||
*Draft: 2026-10-21*
|
||||
|
||||
## Self-Check: PASSED
|
||||
|
||||
All eight commits and the four created files exist; the draft status line is present once.
|
||||
@@ -5,3 +5,8 @@ Out-of-scope discoveries logged by executors; not fixed in this phase.
|
||||
## From plan 19.5-03
|
||||
|
||||
- **fonoteka.go `TestPhase09SecurityRoutes` (plugins/golem15/fonoteka/admin_phase09_security_test.go:52) fails in the Docker-backed run.** The admin route inventory does not list five `summercms.cabana` routes the framework working tree now mounts: `GET`/`PUT /plytadmin/api/v1/account/preferences`, `GET`/`POST /plytadmin/api/v1/{vendor}/{plugin}/{controller}/reorder` and `GET /plytadmin/api/v1/{vendor}/{plugin}/{controller}/singleton`. Every reported route belongs to cabana, none to golem15.user, so the sm-user-plugin bump does not cause it; the inventory needs the framework's newer admin routes (the pattern of fonoteka's earlier "list the post-Phase 9 cabana admin routes" commit).
|
||||
|
||||
## From plan 19.5-05
|
||||
|
||||
- **sm-user-plugin `updates/postgres_test.go` `testShort` scans `os.Args` for `-test.short`.** `go test` passes `-test.short=true`, so `go test -short ./plugins/golem15/user/...` still starts the Postgres container for the `updates` package (the pitfall CLAUDE.md names). Pre-existing, not caused by plan 05; the fix is `flag.Parse()` and `testing.Short()` in its `TestMain`.
|
||||
- **Docker host: published container ports do not answer on `[::1]`.** On this host testcontainers resolves `localhost`, pgx dials `[::1]` first and times out, and every Docker-backed suite fails with `postgres unavailable`. Plan 05 ran the Docker stages with `TESTCONTAINERS_HOST_OVERRIDE=127.0.0.1`; Task 5's `--all` needs the same until the host's IPv6 loopback forwarding is fixed.
|
||||
|
||||
Reference in New Issue
Block a user