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

docs(19.5): proof of address in the security review and validation map

Browse Source 
- T-19.5-38 describes the proof as email_verified_at; T-19.5-39..46 added;
  T-19.5-26 closed by OAuthCallbackView.test.ts; CR-02 recorded as open
- validation rows 19.5-05-01..07 and D-26, D-27, D-28
This commit is contained in:
Jakub Zych
2026-10-21 14:46:38 +02:00
parent 7e4387311e
commit 8c7794efa2
2 changed files with 37 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

31
.planning/phases/19.5-social-login-in-sm-user-plugin/19.5-SECURITY-REVIEW.md
View File

@@ -3,8 +3,8 @@
**Date:** 2026-10-21
**Level:** OWASP ASVS L1 (RESEARCH "Security Domain"), block on high; D-11 asks for this review of the matching and state code.
**Scope:** `cooler.Cache.Pull` (summercms.go `modules/cooler`), this phase's sm-user-plugin additions (provider source, row-locked unlink, exported sign-in helpers), the new `sm-sociallogin-plugin` (registry, goth wrappers, state cookie, callback rules, one-time codes, matching, registration, linking, settings), the adoption in sm-demo-app, sm-bm-app and fonoteka.go, and this plan's gate.
**Threats reviewed:** T-19.5-01..T-19.5-37 and T-19.5-SC from the threat models of plans 01, 02, 03 and 04, plus RESEARCH's Security Domain table.
**Result:** every threat is closed by a test that fails when the mitigation is removed, or accepted with a reason (process controls). Two defects found by the new tests were fixed in their own commits (one of them on T-19.5-12). One new finding, T-19.5-38 (pre-account takeover through an account created from an unverified provider email, the PHP behaviour D-10 ported), was raised at the Task 6 checkpoint; the user chose option (b), and it is closed by commit d7ee28d with `TestPreAccountTakeover`.
**Threats reviewed:** T-19.5-01..T-19.5-37 and T-19.5-SC from the threat models of plans 01, 02, 03 and 04, plus RESEARCH's Security Domain table; T-19.5-39..T-19.5-46 from plan 05 (gap closure, D-26..D-28).
**Result:** every threat is closed by a test that fails when the mitigation is removed, or accepted with a reason (process controls). Two defects found by the new tests were fixed in their own commits (one of them on T-19.5-12). One new finding, T-19.5-38 (pre-account takeover through an account created from an unverified provider email, the PHP behaviour D-10 ported), was raised at the Task 6 checkpoint; the user chose option (b), and it is closed by commit d7ee28d with `TestPreAccountTakeover`. Plan 19.5-05 closes its password-registration variant (19.5-REVIEW CR-01, T-19.5-39): the owner-side proof is now `users.email_verified_at`, which only the mailed activation code and a completed reset-code password reset write, instead of a self-set password. 19.5-REVIEW CR-02 stays open (section "Open findings").
Repositories and abbreviations: **F** = summercms.go, **U** = sm-user-plugin (`fonoteka.go/plugins/golem15/user`), **S** = `sm-sociallogin-plugin`, **D** = sm-demo-app, **B** = sm-bm-app (with `plugins/jz/bm` and `frontend/`), **A** = the fonoteka.go workspace.
@@ -21,7 +21,7 @@ Repositories and abbreviations: **F** = summercms.go, **U** = sm-user-plugin (`f
|--------|------|--------|----------|-----|
| 21f3fe5 | S | T-19.5-12 | `TestRegisterComplete/concurrent_submit_one_winner` | Two submissions of one pending code that both reached account creation raced on the `users.email` unique index: the loser answered **500** instead of 410. Account creation now takes transaction-scoped advisory locks on the provider identity and the email (`lockCreation`), and a completion that gets them after a winner committed finds the identity and returns `ErrPendingConsumed` (410), writing nothing. |
| c99f5f9 | S | T-19.5-16 (settings write path) | `TestSettings/admin_json_hides_secrets` | Every save of the admin settings page answered 500: `Settings.Rules()` used `regex:`, which lagoon's model validation does not support, so the encrypted secrets could not be managed from the admin at all. The tenant format moved to `Settings.BeforeValidate` (422 on `microsoft_tenant`); README updated. |
| d7ee28d | S | T-19.5-38 | the review (finding), pinned by `TestPreAccountTakeover` | A verified sign-in linked onto a password-less account created from an unverified provider email (pre-account takeover). The callback and the pending completion now auto-link a verified email only onto an account with a self-set password (`ownerVerifiedEmail`); otherwise pending `unverified_match` and 422 `email_exists_link_account`. A password reset through the mailed code proves the address. Deliberate divergence from PHP (user decision, option b). |
| d7ee28d | S | T-19.5-38 | the review (finding), pinned by `TestPreAccountTakeover` | A verified sign-in linked onto a password-less account created from an unverified provider email (pre-account takeover). The callback and the pending completion now auto-link a verified email only onto an account with a self-set password (`ownerVerifiedEmail`); otherwise pending `unverified_match` and 422 `email_exists_link_account`. Deliberate divergence from PHP (user decision, option b). 19.5-05 superseded the self-set-password test with `users.email_verified_at` (T-19.5-39). |
All three are production changes after plan 02's release (v0.1.0); they reach the applications through the v0.1.1 release (T-19.5-36).
@@ -46,7 +46,7 @@ The rest of the pins are this plan's unit tests (Tasks 2 to 4); the register nam
| T-19.5-07 | callback (login CSRF, injected code) | Sealed per-browser state cookie; empty state refused; `subtle.ConstantTimeCompare`; provider bound by the cookie name (AAD) and the `Provider` field; cookie cleared on every outcome. | S `controllers/callback.go:27`, `:35-45`; `classes/statecookie.go:134-162` | `TestStateCookie/callback` (missing cookie, empty and unequal state, replay after clear, another provider's callback, cleared on success, provider error, failed exchange, expired code) (S) | closed |
| T-19.5-08 | `SealState` / `OpenState` | AES-256-GCM, fresh nonce, HKDF purpose key separate from lagoon's, AAD the cookie name, version and iat (600 s, 60 s skew) checked, 3800-byte bound. | S `classes/statecookie.go:93-129`, `:180-220` | `TestStateCookie/refused` (16 cases), `age_window`, `seal_limits`, `key_domain_separated` (new) (S) | closed |
| T-19.5-09 | `NormalizeFrontendCallback`, `NormalizeReturnTo` | Exact-host allow-list, fixed path, the target rebuilt from scheme, host and port; return_to local only; invalid input falls back to `default_callback`. The callback uses the target sealed at the start, never a request value. | S `classes/callback.go:56-107`; `controllers/callback.go:36-39` | `TestNormalizeFrontendCallback` (44 subtests), `TestNormalizeReturnTo` (22 subtests) (S); `TestLinkSmoke/bad_callback_falls_back` (S) | closed |
| T-19.5-10 | email matching (account takeover) | Verified only where the provider asserts it: Google a boolean `verified_email`/`email_verified`, GitHub the primary verified `/user/emails` address, Facebook and Microsoft never; an unverified match goes pending `unverified_match`; case-insensitive match. | S `classes/providers_goth.go:126-254` (Facebook `:154`, Microsoft `:251`); `classes/flow.go:92-101` | `TestEmailVerified`, `TestNOAuth` (Microsoft mail of an existing user in four tenants: pending, no identity, completion 422), `TestMatching`, `TestPHPSocialAuthGate` (S) | closed (the creation path: T-19.5-38, closed) |
| T-19.5-10 | email matching (account takeover) | Verified only where the provider asserts it: Google a boolean `verified_email`/`email_verified`, GitHub the primary verified `/user/emails` address, Facebook and Microsoft never; an unverified match goes pending `unverified_match`; case-insensitive match. | S `classes/providers_goth.go:126-254` (Facebook `:154`, Microsoft `:251`); `classes/flow.go:92-101` | `TestEmailVerified`, `TestNOAuth` (Microsoft mail of an existing user in four tenants: pending, no identity, completion 422), `TestMatching`, `TestPHPSocialAuthGate` (S) | closed (the creation paths: T-19.5-38 and T-19.5-39, closed) |
| T-19.5-11 | `RegisterComplete` (pending hijack) | Completion re-checks identity and email match; `email_verified` false or absent gives 422 `email_exists_link_account`, never a login; payloads sealed. | S `classes/flow.go:175-228` | `TestRegisterComplete/email_exists_link_account_keeps_code`, `TestPHPPendingRegistrationTakeover` (incl. a payload sealed without the flag) (S) | closed |
| T-19.5-12 | one-time codes (replay, race) | 64-character `crypto/rand` codes sealed under the cache key, consumed with `Pull`; TTLs 5 min, 10 min, 60 s; register-complete decides before it pulls; concurrent completions are serialised (21f3fe5). | S `classes/codes.go:81-224`; `controllers/register_complete.go:80-87`; `classes/registration.go:192` | `TestCodes` (incl. moved values, 32 concurrent pulls), `TestOAuthComplete`, `TestRegisterComplete/concurrent_submit_one_winner` (S) | fixed in commit 21f3fe5 (with its test) |
| T-19.5-13 | beta gate (D-12) | Constant-time comparison on both creation paths; the start request's key carried in the sealed cookie and pending payload. | S `classes/registration.go:55-67`; `controllers/register_complete.go:71`, `:116-125` | `TestBetaGate` (S) | closed |
@@ -62,25 +62,39 @@ The rest of the pins are this plan's unit tests (Tasks 2 to 4); the register nam
| T-19.5-23 | remote operations of plan 02 | Blocking go-ahead, fetch and range check before every push, never forced, no deploy. | 19.5-02-SUMMARY "Remote operations" | release control | accepted: a process control, recorded in 19.5-02-SUMMARY |
| T-19.5-24 | bm SPA fallback | `/oauth` among the reserved prefixes of the static handler and the service worker. | B `plugins/jz/bm/static.go`; `frontend/src/pwa/sw.js` | `TestStaticReservedPaths` (B, gate `--bm`); `pwa.test.ts` (B) | closed |
| T-19.5-25 | committed configuration and env templates | Credentials only in `.env` or the encrypted settings. | D, B `config/`, `deploy/` | grep in this review: no non-empty `CLIENT_SECRET=` or `client_secret:` in D or B config and templates | accepted: verified by grep; no runtime behaviour to test |
| T-19.5-26 | bm callback view (code in history) | The view replaces the route before redeeming the code once; the server consumes it with `Pull`, so a code left in history is dead. | B `frontend/src/views/OAuthCallbackView.vue`; S `controllers/complete.go:24` | `TestOAuthComplete/once_with_shape` (second use 410) (S); `oauth.test.ts` (B) | closed for the server half; the view's history handling is a browser behaviour left to `/msd-verify-work` (manual row in VALIDATION.md) |
| T-19.5-27 | Microsoft on bm (nOAuth, tenant) | Microsoft never verified, whatever the tenant; existing students link through the ticket. | S `classes/providers_goth.go:225-254` | `TestNOAuth` (four tenants) (S); `TestSocialLoginMounted` (B) | closed (creation path: T-19.5-38, closed) |
| T-19.5-26 | bm callback view (code in history) | The view replaces the route before redeeming the code once; the server consumes it with `Pull`, so a code left in history is dead. | B `frontend/src/views/OAuthCallbackView.vue`; S `controllers/complete.go:24` | `TestOAuthComplete/once_with_shape` (second use 410) (S); `oauth.test.ts` (B); `src/views/OAuthCallbackView.test.ts` (B, 5fd0cc3: the real view mounted with vue's `createRenderer` and a memory-history router; the code is out of the route when the single `completeOAuth` starts, a second mount does not redeem again, `?error=` shows in the alert, a pending registration replaces to `oauth-register`; D-28) | closed (the view half by `OAuthCallbackView.test.ts`, plan 19.5-05) |
| T-19.5-27 | Microsoft on bm (nOAuth, tenant) | Microsoft never verified, whatever the tenant; existing students link through the ticket. | S `classes/providers_goth.go:225-254` | `TestNOAuth` (four tenants) (S); `TestSocialLoginMounted` (B) | closed (the creation paths: T-19.5-38 and T-19.5-39, closed) |
| T-19.5-28 | link ticket in the bm profile flow | Ticket from an authenticated POST, 60 s, single use, the only value in the URL. | S `controllers/link_ticket.go`; B `frontend/src/lib/oauth.ts` | `TestLinkTicket` (S); `oauth.test.ts` (B) | closed |
| T-19.5-29 | pushes of shared repositories (plan 03) | Push only under plan 02's reply, plugin before pointer, after fetch and a range check. | 19.5-03-SUMMARY "Pushes" | release control | accepted: a process control, recorded in 19.5-03-SUMMARY |
| T-19.5-30 | fonoteka regression from D-13 | fonoteka's vet and short suite at the new pointer; the Docker-backed user plugin suite. | A | gate `--fonoteka` (F); `TestOAuthIdentitiesDestroyRule` (U) | closed (the unrelated `TestPhase09SecurityRoutes` failure is outside this phase, deferred-items.md) |
| T-19.5-31 | gate false green | go test -json detector fails on fail, skip (subtests too), zero tests, "no tests to run", a missing name, a prefix or a subtest standing in for a name; the vitest detector refuses skips and a missing `oauth.test.ts`; NOT RUN docker is loud and fails with `PHASE195_REQUIRE_DOCKER=1`. | F `scripts/check-phase19.5.sh` | `scripts/check-phase19.5.sh --self-test` (F) | closed |
| T-19.5-31 | gate false green | go test -json detector fails on fail, skip (subtests too), zero tests, "no tests to run", a missing name, a prefix or a subtest standing in for a name; the vitest detector refuses skips and a missing required file (`oauth.test.ts`, `OAuthCallbackView.test.ts`); NOT RUN docker is loud and fails with `PHASE195_REQUIRE_DOCKER=1`. | F `scripts/check-phase19.5.sh` | `scripts/check-phase19.5.sh --self-test` (F) | closed |
| T-19.5-32 | race tests (D-13, D-08) | Forced with explicit transactions and start barriers, both orders, repeated. | F `modules/cooler/pull_test.go`; U `oauth_identities_rule_test.go` | the mutation checks above (F, U) | closed |
| T-19.5-33 | completeness of this review | Every T-19.5 ID of plans 01-04 appears here with a test or an accepted reason. | this document | the plan's acceptance count | closed |
| T-19.5-34 | test fixtures and logs | Test-only literal secrets; provider HTTP faked through `ProviderConfig.HTTPClient`; the fake transport refuses hosts outside its table and the tests check every request stayed on the provider's hosts. | S `providers_goth_test.go`, `settings_test.go` | `TestGothProviders/fake_transport_refuses_unknown_hosts`, `TestSettings/no_secret_in_logs` (S) | closed |
| T-19.5-35 | PHP map honesty | The map is checked against the list, the live PHP files and the Go test sources of S and U. | S `php_method_map_test.go` | `TestPHPMethodMap` (mutation-checked) (S) | closed |
| T-19.5-36 | post-release fixes | The plan's "Post-release fixes" table lists 21f3fe5, c99f5f9 and d7ee28d against every pin that lacks them; the blocking-human Task 6 asks for the go-ahead; Task 7 releases in D-21 order and checks every pin with `merge-base --is-ancestor`. | 19.5-04 Tasks 5-7 | Task 7's acceptance criteria | release control: Task 6 reply "release"; v0.1.1 and the pointer bumps checked by Task 7's acceptance criteria (19.5-04-SUMMARY) |
| T-19.5-37 | remote operations of Task 7 | Only after the Task 6 reply; fetch and range check before every push; never forced or deployed; only `plugins/golem15/user` staged in fonoteka.go. | 19.5-04 Task 7 | Task 7's acceptance criteria | release control: recorded with Task 7's acceptance criteria |
| T-19.5-39 | sociallogin `ownerVerifiedEmail` (password-registration pre-account takeover, 19.5-REVIEW CR-01) | A verified provider email auto-links only onto an account whose `users.email_verified_at` is set; only the mailed activation code and a completed reset-code password reset write it. An auto-mode `/register` account, an admin-created or admin-activated account and every account from before sm-user-plugin v0.1.5 go pending `unverified_match`; the completion answers 422 `email_exists_link_account` and keeps the code. No backfill. | S `classes/flow.go:266` `ownerVerifiedEmail` (used by `HandleCallback` `:98` and `CompletePending` `:203`), 3bb5650; U `classes/codes.go:119` `VerifyActivationCode`, `controllers/api_controller.go:290` `ResetPassword` (add1f7a) | `TestOwnerVerifiedEmail/register_auto_then_google_pending`, `register_action_pending`, `legacy_account_pending`, `mail_activated_by_code_links`, `reset_code_password_links`; `TestPreAccountTakeover/after_mailbox_proof_links` (drives `/forgot-password` and `/reset-password`) (S) | closed (RED: the self-set-password rule fails `register_auto_then_google_pending`) |
| T-19.5-40 | U `Update` and `FormBeforeUpdate` (proof carried over an email change) | An email change sets `email_verified_at` to NULL in the same write; a change of letter case only (`strings.EqualFold`) keeps it. | U `controllers/api_controller.go:444` `Update` (`:485`), `controllers/users_admin_controller.go:217` `FormBeforeUpdate` with the shared `storedEmail` (a99cb46) | `TestEmailVerifiedAt/update_email_change_clears`, `update_same_email_keeps`, `TestAdminEmailVerifiedAt/admin_email_change_clears`, `admin_save_keeps_proof` (U); `TestOwnerVerifiedEmail/email_change_drops_proof` (S) | closed (RED: the controllers before a99cb46 fail exactly the two `*_email_change_clears` cases) |
| T-19.5-41 | `/register` and `/update` bodies (mass assignment of the proof) | `EmailVerifiedAt` is not in `Fillable()`; `Update` writes an explicit column map. | U `models/user.go:50`; `controllers/api_controller.go:444` | `TestEmailVerifiedAt/register_ignores_submitted_proof`, `update_ignores_submitted_proof` (U) | closed |
| T-19.5-42 | non-mailbox paths treated as proof (`/register` auto and admin modes, `RegisterUser`, admin create, admin Activate action, change-password, social sign-in) | None of these paths writes the column. | U `controllers.Register`, `classes.RegisterUser`, `usersAdminController.FormBeforeCreate`, `classes.ActivateUsers`, `controllers.ChangePassword`; S `classes/registration.go` (account creation) | `TestEmailVerifiedAt/register_auto_no_proof`, `register_admin_mode_no_proof`, `change_password_no_proof`, `TestAdminEmailVerifiedAt/admin_create_no_proof`, `admin_activate_action_no_proof` (U); `TestOwnerVerifiedEmail/admin_activation_not_proof`, `unverified_provider_never_links_proven_account`, `TestPreAccountTakeover` (a social-created account has no proof) (S) | closed |
| T-19.5-43 | user API payloads and admin JSON (contract drift in a core plugin) | `json:"-"`; `apiArray`, the admin `fields.yaml` and `columns.yaml`, routes, config and lang unchanged since v0.1.4. | U `models/user.go:50` | the payload-key assertion in `TestEmailVerifiedAt` (no `email_verified_at` in any answer), `TestAdminEmailVerifiedAt/admin_json_hides_proof` (U); gate `--userplugin` additive check (F) | closed |
| T-19.5-44 | migration `202610210001_add_users_email_verified_at` (data rewrite, broken upgrade) | `ALTER TABLE users ADD COLUMN IF NOT EXISTS email_verified_at TIMESTAMPTZ NULL`, no row written, no-op rollback. The gate accepts exactly this one added file under `updates/` and refuses a migration that writes or drops data. | U `updates/202610210001_add_users_email_verified_at.go:20-22`; F `scripts/check-phase19.5.sh` `USER_ADDED_MIGRATIONS`, `refused_migration_changes`, `migration_writes_rows` (7170c5b) | `TestEmailVerifiedAtMigration` (`column_shape`, `no_backfill`, `existing_column_kept`, `rollback_keeps_column`, `history_once`), `TestCreateOrganisationsMigration` (U); `--self-test` migration cases (allowed add, modified base, other add, delete, rename, planted row update, comment-only word) (F) | closed |
| T-19.5-45 | scripts/check-phase19.5.sh (false green for the plan 05 tests) | The new Go tests are required by name (`DOCKER_USER_UPDATES_TESTS` and the root, sociallogin lists); `detect_vitest` takes `REQUIRE_FILES` and `run_bm` requires `src/views/OAuthCallbackView.test.ts` next to `src/lib/oauth.test.ts`. | F `scripts/check-phase19.5.sh` (7170c5b, 30e0cd9, 7e43873) | `--self-test` (`missing-view-file` exits 5, the migration-check cases) (F) | closed |
| T-19.5-46 | release and pointer bumps of plan 05 (Task 7) | Only after the blocking-human Task 6 reply; the user pointer moves before sociallogin's; each application's gate stage after every bump; fetch and `origin/master..master` check before every push; never forced or deployed; fonoteka.go and summercms.go not pushed. | 19.5-05 Tasks 6-7 | Task 7's acceptance criteria | release control: awaiting the Task 6 reply |
| T-19.5-SC | module installs | goth v1.82.0 approved in plan 01 Task 4; no module installed in plan 04 (S `go.mod` and `go.sum` unchanged since adfcfbf); goth imported only by `classes/providers_goth.go`; no gorilla/sessions or gopkg.in/yaml.v3 in `go list -deps .`. | S `go.mod`; `classes/providers_goth.go:13-18` | gate `--sociallogin` (deps and importer checks) (F) | closed |
## New finding
| Threat | Category | Component | Severity | Description | Status |
|--------|----------|-----------|----------|-------------|--------|
| T-19.5-38 | Elevation of privilege | account creation from an unverified provider email (`HandleCallback` with `action=register`, `CompletePending`) | high | An account is created **activated** with an email the provider did not verify (Facebook, Microsoft, a Google answer without the flag). An attacker who controls such a provider account with a victim's address (any Entra tenant can set `mail`; nOAuth) creates the account before the victim does; when the victim later signs in with a verifying provider (Google, GitHub), the verified match linked that identity onto the attacker's account, and the attacker kept access through the original identity (pre-account takeover, classic-federated merge). PHP's SocialAuth behaves the same (D-10). Mitigation (user decision at the Task 6 checkpoint, option b, a deliberate divergence from PHP): the callback and the pending completion auto-link a verified email only onto an account with a self-set password (S `classes/flow.go` `ownerVerifiedEmail`, used in `HandleCallback` and `CompletePending`); a password-less account goes pending `unverified_match` and its completion answers 422 `email_exists_link_account`; the owner proves the address with the mailed password reset (which sets `has_self_set_password`), after which verified providers link again. Test: `TestPreAccountTakeover` (S; mutation-checked: an always-true `ownerVerifiedEmail` fails it). | fixed in commit d7ee28d (with its test) |
| T-19.5-38 | Elevation of privilege | account creation from an unverified provider email (`HandleCallback` with `action=register`, `CompletePending`) | high | An account is created **activated** with an email the provider did not verify (Facebook, Microsoft, a Google answer without the flag). An attacker who controls such a provider account with a victim's address (any Entra tenant can set `mail`; nOAuth) creates the account before the victim does; when the victim later signs in with a verifying provider (Google, GitHub), the verified match linked that identity onto the attacker's account, and the attacker kept access through the original identity (pre-account takeover, classic-federated merge). PHP's SocialAuth behaves the same (D-10). Mitigation (user decision at the Task 6 checkpoint, option b, a deliberate divergence from PHP): the callback and the pending completion auto-link a verified email only onto an account whose owner proved the address (S `classes/flow.go` `ownerVerifiedEmail`, used in `HandleCallback` and `CompletePending`); an account without proof goes pending `unverified_match` and its completion answers 422 `email_exists_link_account`. The proof is `users.email_verified_at` (19.5-05, D-27), written only by the mail activation code and a completed reset-code password reset; d7ee28d had used a self-set password, which an auto-mode `/register` also sets (CR-01, T-19.5-39). After the owner proves the address, verified providers link again. Test: `TestPreAccountTakeover` (S; mutation-checked: an always-true `ownerVerifiedEmail` fails it), `TestOwnerVerifiedEmail` (S). | fixed in d7ee28d; the password-registration variant fixed in 19.5-05 (T-19.5-39) |
## Open findings
| Finding | Severity | Description | Status |
|---------|----------|-------------|--------|
| 19.5-REVIEW CR-02 | high (review) | An account pre-created from an unverified provider email (sociallogin creates it activated, S `classes/registration.go`) keeps the attacker's identity after the real owner proves the address with a reset-code password reset: the reset stamps `email_verified_at` and sets a password but leaves `golem15_user_oauth_identities` alone, so the attacker keeps signing in through the pre-planted Microsoft or Facebook identity. | open, no user decision; not covered by D-26..D-28 |
## Log calls
@@ -123,6 +137,7 @@ Every non-test `slog` call of S and of this phase's U changes, with what it logs
- No JWT, provider token, secret, code or `RetrieveError` body reaches a log line: section "Log calls"; `TestSettings/no_secret_in_logs`, `TestGothProviders/expired_code_without_body`.
- The unlink lock is taken before the DELETE: U `controllers/oauth_identities.go:102`; `TestOAuthIdentitiesDestroyRace` (mutation-checked).
- goth has a timeout on every provider: `classes/credentials.go:32`, `classes/providers_goth.go:116-121`; `TestGothProviders/timeout`.
- The proof follows the address: an email change through `/update` or the admin Users form clears `email_verified_at` in the same write, a case-only change keeps it (U `controllers/api_controller.go:485`, `controllers/users_admin_controller.go:217`); `TestEmailVerifiedAt/update_email_change_clears`, `TestAdminEmailVerifiedAt/admin_email_change_clears`, `TestOwnerVerifiedEmail/email_change_drops_proof`.
## Accepted residuals (low)

16
.planning/phases/19.5-social-login-in-sm-user-plugin/19.5-VALIDATION.md
View File

@@ -39,7 +39,7 @@ validated: "2026-10-21"
## Per-Task Verification Map
One row per task of plans 01-04. Gate stages are modes of `scripts/check-phase19.5.sh`; repositories: F summercms.go, U sm-user-plugin, S sm-sociallogin-plugin, D sm-demo-app, B sm-bm-app, A fonoteka.go.
One row per task of plans 01-05. Gate stages are modes of `scripts/check-phase19.5.sh`; repositories: F summercms.go, U sm-user-plugin, S sm-sociallogin-plugin, D sm-demo-app, B sm-bm-app, A fonoteka.go.
| Task | Decision | Test names | Gate stage | Status |
|------|----------|------------|------------|--------|
@@ -64,6 +64,13 @@ One row per task of plans 01-04. Gate stages are modes of `scripts/check-phase19
| 19.5-04-05 | D-11 review, SC-4 | `19.5-SECURITY-REVIEW.md` (every T-19.5 threat to a test or an accepted reason); the whole gate | `--all` | ✅ green |
| 19.5-04-06 | D-21 (post-release go-ahead) | remote operation: Task 7's acceptance criteria (no patch tag before the reply; the reply recorded verbatim) | (none: remote operation) | ⬜ awaiting the Task 6 reply |
| 19.5-04-07 | D-21 (post-release release) | remote operation: Task 7's acceptance criteria (`merge-base --is-ancestor` per fix and pin, tags on the remotes, `origin/master..master` empty) and `--sociallogin`, `--demo`, `--bm`, `--fonoteka` at the new pins | `--sociallogin`, `--demo`, `--bm`, `--fonoteka` | ⬜ awaiting the Task 6 reply |
| 19.5-05-01 | D-26, D-27 (tracer) | `TestOwnerVerifiedEmail/register_auto_then_google_pending`, `mail_activated_by_code_links`, `reset_code_password_links`; `TestPreAccountTakeover` (S); `TestEmailVerifiedAtMigration`, `TestCreateOrganisationsMigration` (U); migration allow-list self-test cases (F) | `--self-test`, `--userplugin`, `--sociallogin`, `--docker` | ✅ green |
| 19.5-05-02 | D-26, D-27 (writers, clearing) | `TestEmailVerifiedAt` (12 subtests), `TestAdminEmailVerifiedAt` (6 subtests), `TestEmailVerifiedAtMigration` (5 subtests) (U) | `--userplugin`, `--fonoteka`, `--docker` | ✅ green |
| 19.5-05-03 | D-26, D-27 (owner proof table, wording) | `TestOwnerVerifiedEmail` (8 subtests), `TestPreAccountTakeover`, `TestPHPPendingRegistrationTakeover`, `TestPHPMethodMap` (S) | `--sociallogin`, `--docker` | ✅ green |
| 19.5-05-04 | D-28 | vitest `src/views/OAuthCallbackView.test.ts` (5 tests), `npm run typecheck`, package.json and lockfile unchanged (B); `--self-test` `missing-view-file` (F) | `--self-test`, `--bm` | ✅ green |
| 19.5-05-05 | D-27 wording, gate | `19.5-SECURITY-REVIEW.md` (T-19.5-39..46, CR-02 open), this map; the whole gate with `PHASE195_REQUIRE_DOCKER=1` | `--all` | ✅ green |
| 19.5-05-06 | D-21, D-27 (release go-ahead) | remote operation: Task 7's acceptance criteria (no tag on either plugin before the reply; the reply recorded verbatim) | (none: remote operation) | ⬜ awaiting the Task 6 reply |
| 19.5-05-07 | D-21, D-27, SC-3 (release) | remote operation: Task 7's acceptance criteria (v0.1.5 and v0.1.2 on the remotes, sociallogin go.mod requires sm-user-plugin v0.1.5, app pins equal the tags, user bump before sociallogin bump, `origin/master..master` empty) and `--sociallogin`, `--demo`, `--bm`, `--fonoteka` at the new pins | `--sociallogin`, `--demo`, `--bm`, `--fonoteka` | ⬜ awaiting the Task 6 reply |
Decision coverage (from RESEARCH § Validation Architecture):
@@ -87,6 +94,9 @@ Decision coverage (from RESEARCH § Validation Architecture):
| D-20 | route middleware throttles | `TestRouteMiddleware` (S) | `--docker` | ✅ green |
| PHP map | one Go test per PHP social-auth test case (38 methods) | `TestPHPMethodMap` and the five `TestPHP*` ports (S) | `--sociallogin`, `--docker` | ✅ green |
| D-21 | demo and bm boot with sociallogin; bm `/oauth` reserved from the SPA fallback; fonoteka green at the new pin | `TestBootAllPlugins`, `TestSocialLoginRedirect`, `TestSocialLoginCoexistsWithOAuthServer` (D); `TestSocialLoginMounted`, `TestStaticReservedPaths` (B); fonoteka short suite (A) | `--demo`, `--bm`, `--fonoteka` | ✅ green |
| D-26 | a verified email auto-links only onto an account with recorded proof; `/register` auto and admin modes, admin create and Activate, social sign-in and legacy accounts go pending; mail activation and the reset code link | `TestOwnerVerifiedEmail` (8 subtests), `TestPreAccountTakeover` (S); `TestEmailVerifiedAt`, `TestAdminEmailVerifiedAt` (U) | `--docker` | ✅ green |
| D-27 | `users.email_verified_at` additive (nullable column, no backfill, no-op rollback), two writers, cleared on an email change, not fillable, not in any payload; READMEs and the security review describe proof as `email_verified_at` | `TestEmailVerifiedAtMigration`, `TestCreateOrganisationsMigration`, `TestEmailVerifiedAt`, `TestAdminEmailVerifiedAt` (U); `TestPHPMethodMap` (S); migration allow-list and additive check (F) | `--userplugin`, `--sociallogin`, `--docker` | ✅ green |
| D-28 | the bm callback view drops `?code=` before the single redeem, never redeems twice, shows `?error=`, replaces to `oauth-register` for a pending registration, explains an expired code; no new npm dependency | vitest `src/views/OAuthCallbackView.test.ts` (B) | `--bm` | ✅ green |
*Status: ⬜ pending · ✅ green · ❌ red · ⚠️ flaky*
@@ -107,6 +117,8 @@ Decision coverage (from RESEARCH § Validation Architecture):
|----------|-------------|------------|-------------------|
| Live sign-in with Google, Facebook, GitHub and Microsoft | D-04, D-21 | Needs real provider app registrations and secrets | Configure credentials in sm-demo-app (and Entra in sm-bm-app), sign in through the SPA, confirm the callback lands and the identity is listed |
The bm callback view's history handling (the code leaves the URL before the single redeem) was a manual check until plan 05; it is now automated by `src/views/OAuthCallbackView.test.ts` (D-28, row 19.5-05-04).
---
## Validation Sign-Off
@@ -118,4 +130,4 @@ Decision coverage (from RESEARCH § Validation Architecture):
- [x] Feedback latency < 60s
- [x] `nyquist_compliant: true` set in frontmatter
**Approval:** validated 2026-10-21 (plan 19.5-04); rows 19.5-04-06 and -07 are remote operations that follow the Task 6 reply.
**Approval:** validated 2026-10-21 (plan 19.5-04; plan 19.5-05 rows added the same day); rows 19.5-04-06 and -07 and 19.5-05-06 and -07 are remote operations that follow their plan's Task 6 reply.