diff --git a/.planning/quick/261022-2zy-user-and-sociallogin-plugin-security-gap/261022-2zy-PLAN.md b/.planning/quick/261022-2zy-user-and-sociallogin-plugin-security-gap/261022-2zy-PLAN.md new file mode 100644 index 00000000..337738a1 --- /dev/null +++ b/.planning/quick/261022-2zy-user-and-sociallogin-plugin-security-gap/261022-2zy-PLAN.md @@ -0,0 +1,392 @@ +--- +phase: quick-261022-2zy +plan: 01 +quick_id: 261022-2zy +slug: user-and-sociallogin-plugin-security-gap +type: execute +mode: quick +wave: 1 +depends_on: [] +autonomous: false +requirements: + - 2zy-1 # /register mass assignment (organisation fields, IP columns, account flags) + - 2zy-2 # no Entra tenant ID in sign-in data + - 2zy-3 # no post-login event; listener errors discarded + - 2zy-4 # offboarding: sessions and API tokens survive deactivate/ban, /refresh never reloads the user + - 2zy-5 # stale organisation_role after a membership change +files_modified: + # sm-user-plugin, worked in the sm-demo-app submodule checkout + - ../sm-demo-app/plugins/golem15/user/models/user.go + - ../sm-demo-app/plugins/golem15/user/controllers/registration.go + - ../sm-demo-app/plugins/golem15/user/controllers/api_controller.go + - ../sm-demo-app/plugins/golem15/user/controllers/organisations_admin_controller.go + - ../sm-demo-app/plugins/golem15/user/controllers/users_admin_controller.go + - ../sm-demo-app/plugins/golem15/user/classes/admin_actions.go + - ../sm-demo-app/plugins/golem15/user/classes/admin_actions_test.go + - ../sm-demo-app/plugins/golem15/user/classes/events.go + - ../sm-demo-app/plugins/golem15/user/register_test.go + - ../sm-demo-app/plugins/golem15/user/offboarding_test.go + - ../sm-demo-app/plugins/golem15/user/admin_organisations_edge_test.go + - ../sm-demo-app/plugins/golem15/user/README.md + # sm-sociallogin-plugin, standalone repo + - ../sm-sociallogin-plugin/classes/flow.go + - ../sm-sociallogin-plugin/classes/provider.go + - ../sm-sociallogin-plugin/classes/providers_goth.go + - ../sm-sociallogin-plugin/classes/codes.go + - ../sm-sociallogin-plugin/classes/registration.go + - ../sm-sociallogin-plugin/classes/events.go + - ../sm-sociallogin-plugin/matching_test.go + - ../sm-sociallogin-plugin/providers_goth_test.go + - ../sm-sociallogin-plugin/fakeprovider_test.go + - ../sm-sociallogin-plugin/signin_event_test.go + - ../sm-sociallogin-plugin/README.md + # summercms.go framework (Task 4, after phase 19.8 has released) + - modules/cabana/relation.go + - modules/cabana/relation_field.go + - modules/cabana/crud.go + - modules/cabana/clear_on_change_test.go + - modules/cabana/README.md + - docs/backend/relation-manager.md + # only after the user's go-ahead (Task 5) + - ../sm-demo-app/plugins/golem15/user (submodule pointer) + - ../sm-demo-app/plugins/golem15/sociallogin (submodule pointer) + - ../sm-bm-app/plugins/golem15/user (submodule pointer) + - ../sm-bm-app/plugins/golem15/sociallogin (submodule pointer) + - ../fonoteka.go/plugins/golem15/user (submodule pointer) + - ../sm-grzybyfunkcjonalne-app/plugins/golem15/user (submodule pointer) + - ../sm-summercmsio-app/plugins/golem15/user (submodule pointer) + +estimate: + tokens: 220000 + raw_tokens: 220000 + tasks: 5 + confidence: low + +must_haves: + truths: + - "/register and controllers.RegisterUser fill only name, surname, email, password, preferred_locale and marketing_consent; body values for organisation_id, organisation_role, created_ip_address, last_ip_address, is_onboarded and must_change_password are ignored, and the IP columns come only from RegisterOptions.IP. models.User.Fillable() is unchanged, so admin forms, form extensions and RegisterUser callers keep working." + - "Deactivation is self-service reversible: a password login or a social sign-in of a deactivated (soft-deleted) account restores it, sends the reactivate mail and issues a fresh working token. Deactivating also ends every earlier session JWT and revokes every personal API token, and those stay dead after the restore." + - "A ban is the hard lock: a banned user (deactivated or not) is refused by password login, by social sign-in on every path (identity, verified email match, link, pending completion) without being restored, and by /refresh; banning ends every earlier session JWT and revokes every personal API token, and unban does not revive them." + - "POST /_user/api/v1/refresh answers 401 {error: Could not refresh token, msg: User not found} for a token whose user is deactivated, banned, or that was issued before the user's tokens_valid_after; a valid refresh still answers 200 and stamps last_seen." + - "A Microsoft (Entra) sign-in stores the tenant ID from the id_token tid claim in the identity's profile_data.tenant_id; other providers' profile_data keeps exactly name, email, avatar." + - "Every successful social sign-in (login, register, link, pending completion) fires classes.SignedInEvent with provider, provider ID, identity row ID, tenant ID, provider email, action and created flag; its listener errors and the user.Register / user.Created listener errors (social and /register) are logged and never fail or undo the sign-in or registration." + - "cabana's RelationContract (hasMany) and FieldRelationContract (belongsTo) accept ClearOnChange; the user plugin uses it so that adding or removing a member in the Organisations members manager, and changing the organisation in the Users form, clears organisation_role unless the same save writes it; deleting an organisation still clears both columns." + artifacts: + - path: ../sm-demo-app/plugins/golem15/user/models/user.go + provides: "RegisterFillable, the explicit registration allow-list" + contains: "func (User) RegisterFillable" + - path: ../sm-demo-app/plugins/golem15/user/classes/admin_actions.go + provides: "RevokeUserAccess; DeactivateUsers and BanUsers revoke sessions and API tokens" + contains: "func RevokeUserAccess" + - path: ../sm-demo-app/plugins/golem15/user/controllers/api_controller.go + provides: "Refresh subject check; /register logs listener errors" + contains: "ThrottleStates" + - path: ../sm-sociallogin-plugin/classes/events.go + provides: "SignedInEvent, SignedInEventName" + contains: "type SignedInEvent struct" + - path: ../sm-sociallogin-plugin/classes/providers_goth.go + provides: "Microsoft code exchange that keeps the id_token and reads tid" + contains: "id_token" + - path: modules/cabana/relation_field.go + provides: "FieldRelationContract.ClearOnChange applied in assignBelongsTo" + contains: "ClearOnChange" + - path: modules/cabana/relation.go + provides: "RelationContract.ClearOnChange applied on hasMany link and unlink" + contains: "ClearOnChange" + key_links: + - from: ../sm-demo-app/plugins/golem15/user/controllers/registration.go + to: ../sm-demo-app/plugins/golem15/user/models/user.go + via: "lagoon.Fill with RegisterFillable() instead of Fillable() at registration.go:73" + pattern: "RegisterFillable\\(\\)" + - from: ../sm-demo-app/plugins/golem15/user/middleware/api_token.go + to: ../sm-demo-app/plugins/golem15/user/classes/admin_actions.go + via: "token.IsUsableAt(now) refuses rows whose revoked_at RevokeUserAccess set" + pattern: "revoked_at" + - from: ../sm-sociallogin-plugin/classes/flow.go + to: ../sm-sociallogin-plugin/classes/events.go + via: "app.Events.Fire(ctx, &SignedInEvent{...}) in signIn and linkIdentity" + pattern: "SignedInEvent\\{" + - from: ../sm-sociallogin-plugin/classes/providers_goth.go + to: ../sm-sociallogin-plugin/classes/flow.go + via: "Profile.TenantID -> upsertIdentity profile_data tenant_id and PendingPayload.TenantID" + pattern: "TenantID" + - from: ../sm-demo-app/plugins/golem15/user/controllers/organisations_admin_controller.go + to: modules/cabana/relation.go + via: "members RelationContract ClearOnChange organisation_role, applied before Save(child) in linkRelated/unlinkRelated" + pattern: "ClearOnChange" + - from: ../sm-demo-app/plugins/golem15/user/controllers/users_admin_controller.go + to: modules/cabana/relation_field.go + via: "organisation FieldRelationContract ClearOnChange organisation_role, applied in assignBelongsTo before the row write" + pattern: "ClearOnChange" +--- + + +Close the five security gaps reported while planning sm-servers-app in the core plugins sm-user-plugin and sm-sociallogin-plugin, plus one small additive framework feature in cabana, without breaking any app that uses them (fonoteka.go, sm-bm-app, sm-demo-app, sm-grzybyfunkcjonalne-app, sm-summercmsio-app). + +1. (2zy-1) /register mass assignment: organisation fields, the IP columns and account flags. +2. (2zy-2) No Entra tenant ID in sign-in data. +3. (2zy-3) No post-login event; register/created listener errors discarded. +4. (2zy-4) Offboarding: sessions and personal API tokens survive deactivate and ban; /refresh never reloads the user. +5. (2zy-5) Stale organisation_role after a membership change. + +Purpose: an application (sm-servers-app first) can let users in through Entra, assign organisations by tenant and email domain, and lock people out with a ban. +Output: local commits in the sm-user-plugin checkout at ../sm-demo-app/plugins/golem15/user, in ../sm-sociallogin-plugin and in summercms.go, each with tests and README/docs in the same commit; then a blocking go-ahead before any push, tag, changelog post or submodule pointer bump. + +Decisions (user-confirmed or planner's discretion; the executor does not revisit them): +- OD-1, confirmed by the user: deactivation is self-service and reversible, a ban is the lock. Password /login already restores a deactivated account (WinterCMS parity, session_test.go TestLogin around :140-156), and social sign-in keeps doing the same through controllers.FinishLogin (registration.go:98-123) — no change in sociallogin's checkCanSignIn and TestSoftDeleted stays as it is. What deactivation adds is the end of earlier sessions and personal API tokens: the user signs in again (which reactivates) and gets a fresh token. /refresh refuses deactivated users so a session never slides silently past a deactivation. A ban is refused everywhere: password login (classes.RejectIfThrottled, existing), social sign-in (checkCanSignIn, existing, now pinned for banned and deactivated accounts), /refresh (new), session JWTs and API tokens (revoked at ban, new). +- OD-2, confirmed: the profile_data key is tenant_id, written only when a tenant is known. +- OD-3, decided: cabana gets an additive ClearOnChange []string on RelationContract (hasMany) and on FieldRelationContract (belongsTo), instead of a GORM hook on models.User. Reason: the change is small (three call sites: the hasMany branches of linkRelated and unlinkRelated in modules/cabana/relation.go around :955 and :1250, and assignBelongsTo in modules/cabana/relation_field.go:648-663; plus two boot validations), declarative, a no-op at its zero value, and it covers both admin paths: the members manager saves the child with Save(child) and no controller hook, and the user form assigns the picked organisation after FormBeforeUpdate (modules/cabana/crud.go: formBeforeUpdate, then assignBelongsTo, then Save), so neither path can be handled in the plugin without a model-wide hook. A declarative column list was preferred over new pact hook interfaces because a hook for the belongsTo path would need the old and the new key, which ClearOnChange compares internally. Cost: the user plugin's stale-role commit needs a summercms tag that contains ClearOnChange, so it is the last user-plugin commit and ships in its own tag (see Task 5). +- Registration allow-list (user's "yes add it"): RegisterUser fills from a new explicit list, User.RegisterFillable(): name, surname, email, password (the hash), preferred_locale, marketing_consent. Not settable from a registration body any more: organisation_id, organisation_role (membership is the admin's or the application's), created_ip_address, last_ip_address (server-set from RegisterOptions.IP only; today a body value survives when opts.IP is empty), is_onboarded (application-owned onboarding state), must_change_password (set by the admin and the require-password-change console command). Already outside Fillable and unchanged: is_activated, activated_at, activation_code(_issued_at), reset_password_code(_issued_at), has_self_set_password, tokens_valid_after, email_verified_at, handle, permissions, last_seen. User.Fillable() itself stays as it is: cabana intersects it with the admin form's fields (and an app's form extensions), and no other caller fills it from client input. Verified frontends send only email, password, password_confirmation, name, surname, marketing_consent plus event-only keys (fonoteka vue app stores/auth.ts:370-378, sm-bm-app frontend api/user.ts:29, vue-demo-app useDemoAuth.ts:34), and every RegisterUser caller passes keys inside the new list (fonoteka onboarding.go:102 and console/dev_seed.go:362, sociallogin registration.go:121-129). +- Listener errors are logged, never returned: user.Register / user.Created in /register and in social account creation, and the new sociallogin.SignedIn event. A committed registration or sign-in is never undone by a listener, as in WinterCMS. +- The login event lives in sociallogin (classes.SignedInEvent): provider, identity and tenant are sociallogin concepts. One event covers login, register, link and pending completion; Created=true marks an account created by that sign-in. +- No framework change for tokens or refresh: front-end personal API tokens live in the user plugin (models/api_token.go:28 table user_api_tokens, guard middleware/api_token.go:35-83); cabana's BackendAPIToken (backend_api_tokens) is for backend administrators and untouched. /refresh is the plugin's controllers.Refresh (api_controller.go:178-215, bouncer.Refresh at :193); bouncer.RefreshAudienceFor would demand an aud claim (PHP-era tokens have none) and knows nothing of bans, so the plugin checks the subject itself. + + + +@~/.claude/msd-core/workflows/execute-plan.md +@~/.claude/msd-core/templates/summary.md + + + +@CLAUDE.md +@.planning/STATE.md +@../sm-demo-app/plugins/golem15/user/README.md +@../sm-sociallogin-plugin/README.md +@modules/cabana/README.md +@docs/backend/relation-manager.md + +Working copies (observed at planning time, 2026-10-22): +- sm-user-plugin: ../sm-demo-app/plugins/golem15/user, detached HEAD 9986342 (= tag v0.1.6 = origin/master); local branch master is stale at 24b16b1 (an ancestor). All five apps mount the plugin at 9986342. sm-demo-app's go.work only replaces the module (it is not in the use list), so every go command in this checkout runs with GOWORK=off; the plugin's go.mod line 176 replaces summercms with ../../../../summercms.go, so it builds against summercms.go's working tree, including other sessions' uncommitted edits (verified: GOWORK=off go vet ./... passes). +- sm-sociallogin-plugin: ../sm-sociallogin-plugin on master at 9fd58db (= origin/master), clean. Its tracked go.work replaces sm-user-plugin with ../fonoteka.go/plugins/golem15/user (9986342), so its own tests run against the published user plugin; no sociallogin change depends on new user-plugin API. +- summercms.go: phase 19.8 (console overhaul) is executing. At planning time the working tree had another session's uncommitted edits in modules/lagoon (19.8-05) and planning files; nothing in modules/cabana, modules/pact or docs/backend. Remaining 19.8 plans: 19.8-05 (bonfire, diary, lagoon, cmd/summer, internal/dev, docs/console, docs/services/logging.md: no overlap with Task 4's files) and 19.8-06 (release v0.4.2, whose range check requires origin/master..master to hold only 19.8-05's code commits and planning documents). A cabana commit before 19.8-06 runs would trip that check, hence Task 4's precondition. Latest tag: v0.4.1. +- Do not edit fonoteka.go or other app checkouts before Task 5's go-ahead. In summercms.go and the plugin repos, stage files by explicit path, never git add -A, never touch .planning/STATE.md or state.json by hand, no co-author tags. + +Source facts the tasks rely on (file:line in the checkouts above): +- user models/user.go:84-92 Fillable(); controllers/registration.go:43-89 RegisterUser fills at :73 and copies opts.IP into the input map at :67-70 only when non-empty; api_controller.go:672 passes readFields(r) (the raw body). /update (api_controller.go:443+) writes only name, surname, email and does not use Fillable. +- cabana admin fill: fillAllowed (modules/cabana/crud.go:1184-1205) intersects Fillable with the form's writable fields; protectedFillKey (:1407) excludes organisation_id; the user form writes organisation_id through the organisation FieldRelationContract with WritableForeignKey (users_admin_controller.go:370-388). fonoteka sets organisation_id and organisation_role with Updates(map)/Exec (onboarding.go:119-135, invitation_service.go:266 and :413, org_provisioner.go:63-70). +- api_controller.go:685-686 discards FireRegisterEvent / FireCreatedEvent errors; sociallogin classes/registration.go:181-183 does the same. +- Members manager: organisations_admin_controller.go:76-90 hasMany contract on users.organisation_id; docs/backend/relation-manager.md:190 (link adopts children whose ForeignKey is NULL, unlink sets it back to NULL). Known-gap test: admin_organisations_edge_test.go:183-213 TestAdminOrganisationRoleOutlivesMembership (assertions at :195-197 and :199-202). Organisation delete already clears both columns (organisations_admin_controller.go:99-117). +- cabana relation code: RelationContract (modules/cabana/relation.go:38-58), validateRelationContract (:479) and validateHasManyContract (:558), linkRelated (:917-975, hasMany Save at :955), unlinkRelated (:1231-1260, hasMany Save at :1250); FieldRelationContract (relation_field.go:22-45), compileFieldRelation (:205), assignBelongsTo (:648-663); the form save order in crud.go: lagoon.Fill(target, fillAllowed(...), projected, ...), validation, formBeforeUpdate/formBeforeCreate, applyPermissionValues, checkRelationScope, checkRelationLocks, assignBelongsTo, Save/Create. Helpers: fieldByColumn (http.go:1204), setModelColumn (relation.go:1332), modelFields (model_fields.go:28). Existing tests: relation_child_test.go (hasMany CRUD, deferred), phase121_form_test.go:582 TestWritableForeignKeySmoke, phase121_relation_lock_test.go:235/:279, phase121_schema_boot_test.go. +- Deactivation: classes/admin_actions.go:95-113 DeactivateUsers only sets deleted_at; BanUsers :140-168 only writes user_throttle; RestoreUsers :117-134; ThrottleStates :21-57. users_admin_controller.go:629-647 wraps DeactivateUsers and fires DeactivatingEvent. users_admin_controller.go:246 already stamps tokens_valid_after on an admin password reset. +- Sessions: classes/user_lookup.go:35-60 PrincipalByID loads scoped and carries TokensValidAfter (the bouncer jwt guard refuses iat before it, modules/bouncer/jwt.go:172); classes/session_user.go:74 does the same for RequestUser. middleware/api_token.go:61-70 accepts any token row with IsUsableAt and a principal. JWT iat has whole-second precision. +- Refresh: api_controller.go:178-215; the "never loads the user row" comment and TouchLastSeen at :204-212; session_test.go:225 TestRefresh (craftToken, bearer, insertUser helpers in session_test.go). bouncer.ErrSubjectRejected text is "User not found" (modules/bouncer/jwt.go:20,144). golang-jwt/jwt/v5 is already a direct requirement of the user plugin. +- sociallogin classes/flow.go: identityUser :328-338 and userByEmail :272-281 load Unscoped; loginWithIdentity :287-307 calls checkCanSignIn :357-371 (activation, ban) before FinishLogin restores; linkIdentity :128-164 issues its own token; upsertIdentity :385-422 writes profile_data :412-416; signIn :311-324. matching_test.go:365 TestSoftDeleted (restore subtests, kept) and :461 TestInactiveAndBanned (inactive and banned on identity, verified_match, link, pending_completion). +- sociallogin Microsoft: providers_goth.go:72-82 Complete calls goth's sess.Authorize, and goth v1.82.0 providers/azureadv2/session.go Authorize keeps only access/refresh token and expiry (the id_token is dropped; Graph /me has no tenant). newMicrosoft :225-254. Profile classes/provider.go:26-41. PendingPayload classes/codes.go:46-77. providers_goth_test.go:38 TestGothProviders (microsoft fake token at :128-130 and :181). settings_test.go:305 TestErrorMessages scans flowErr keys. +- Tests that start Postgres need Docker and skip under -short. If the Docker socket is denied although the user is in the docker group, run the go test command through sudo -u golem (CLAUDE.md). + + + + + + Task 1: sm-user-plugin: explicit registration allow-list, server-set IP columns, logged /register listener errors + ../sm-demo-app/plugins/golem15/user/models/user.go, ../sm-demo-app/plugins/golem15/user/controllers/registration.go, ../sm-demo-app/plugins/golem15/user/controllers/api_controller.go, ../sm-demo-app/plugins/golem15/user/register_test.go, ../sm-demo-app/plugins/golem15/user/README.md + Docker is reachable for testcontainers (directly or through sudo -u golem), and git -C ../sm-demo-app/plugins/golem15/user status --short is empty. + + - TestRegisterIgnoresProtectedFields: seed an organisation row; POST /_user/api/v1/register with email, password, name, surname, preferred_locale "pl", marketing_consent true, plus organisation_id (the seeded id), organisation_role "owner", created_ip_address and last_ip_address "203.0.113.9", is_onboarded true and must_change_password true, answers 200. The stored user has name, surname, preferred_locale and marketing_consent as sent; organisation_id and organisation_role NULL; is_onboarded and must_change_password false; created_ip_address and last_ip_address equal to the request's client address, not 203.0.113.9. + - Direct controllers.RegisterUser with the same fields: with RegisterOptions.IP "198.51.100.7" both IP columns hold it; with an empty IP both are NULL whatever the fields say; the other columns as above. + - Existing TestRegister, TestRegisterEventPayload (the register event payload still carries the client's extra keys, password and confirmation removed), TestRegisterUserPasswordLess and the sociallogin-shaped RegisterUser calls stay green; the admin Users form tests (admin_users_test.go) stay green, proving Fillable() is untouched. + + +Branch setup first (the checkout is detached): in ../sm-demo-app/plugins/golem15/user run git fetch origin, git checkout master and git merge --ff-only origin/master. If origin/master is no longer 9986342, note the new base in the SUMMARY and continue on it. Every go command in this checkout runs with GOWORK=off. + +Allow-list (2zy-1): add func (User) RegisterFillable() []string to models/user.go returning name, surname, email, password, preferred_locale, marketing_consent, with a doc comment naming what a registration body may set and listing what it may not (organisation_id, organisation_role, created_ip_address, last_ip_address, is_onboarded, must_change_password; the activation, reset, proof, handle and permission columns are never fillable). Leave Fillable() (:84-92) as it is and rewrite only its doc comment: the mass-assignment backstop of the admin form (cabana intersects it with the form's fields and keeps organisation_id protected) and of trusted application code; registration uses RegisterFillable. + +RegisterUser (controllers/registration.go:43-89): fill with user.RegisterFillable() instead of user.Fillable() at :73; drop the copy of opts.IP into the input map (:67-70) and instead, after the fill, set CreatedIPAddress and LastIPAddress on the struct from opts.IP when it is not empty, so a body value can never reach those columns. Keep validation, hashing, PasswordLess, Activate and AssignHandle exactly as now. Rewrite the RegisterUser and RegisterOptions doc comments accordingly (fields can set only the RegisterFillable columns; organisation membership and role are set with an explicit update after creation, as an application's onboarding does; IP is the only source of the IP columns). + +Listener errors (2zy-3, user side): at api_controller.go:685-686 replace the discarded results with Error-level logs through the package logger pluginLog (controllers/logger.go): "golem15.user: register listener failed" and "golem15.user: created listener failed", with user_id and err; the registration still succeeds. Update the FireRegisterEvent / FireCreatedEvent doc comments (registration.go:125-150): /register logs that error and continues. + +Tests per the behavior block in register_test.go (reuse sessionApp, disableRegisterThrottle, postJSON; read how the harness sets the request's client address and assert against that). + +README.md (same commits as the code): the registration part of Features/Usage lists what a /register body can and cannot set and that the IP columns are server-set; the Extension events bullet (:39) says /register logs listener errors and continues and that RegisterEvent.Payload is untrusted client input (it still carries every extra key); the RegisterUser / RegisterOptions / FireRegisterEvent / FireCreatedEvent entries (:170-178) match the new comments; add RegisterFillable to the models API list; update the Testing section. + +Commits in the user-plugin checkout, explicit paths, no co-author tags, conventional style as in the repo log: (a) fix: registration fills only an explicit allow-list and never takes the IP columns from the body (models/user.go, registration.go, register_test.go, README); (b) fix: log /register listener failures instead of discarding them (api_controller.go, registration.go comments, README). go vet and the full go test before each commit. Do not push. + + + cd /media/nvme/dev/golem15/summercms.io/summercms/sm-demo-app/plugins/golem15/user && GOWORK=off go vet ./... && GOWORK=off go test -count=1 -run 'TestRegister|TestAdminUser' ./... && GOWORK=off go test -count=1 ./... && sed -n '/^func (User) Fillable/,/^}/p' models/user.go | grep -c 'organisation_role' + + Registration fills only RegisterFillable columns and takes the IP columns only from RegisterOptions.IP; Fillable() still lists its twelve columns (the grep prints 1); /register listener failures are logged; the whole user-plugin suite is green; two commits on master in the user-plugin checkout; nothing pushed. + + + + Task 2: Offboarding: deactivate and ban end sessions and API tokens, /refresh refuses revoked subjects (user plugin); bans pinned and semantics documented (both plugins) + ../sm-demo-app/plugins/golem15/user/classes/admin_actions.go, ../sm-demo-app/plugins/golem15/user/classes/admin_actions_test.go, ../sm-demo-app/plugins/golem15/user/classes/events.go, ../sm-demo-app/plugins/golem15/user/controllers/api_controller.go, ../sm-demo-app/plugins/golem15/user/offboarding_test.go, ../sm-demo-app/plugins/golem15/user/README.md, ../sm-sociallogin-plugin/matching_test.go, ../sm-sociallogin-plugin/README.md + + - classes: RevokeUserAccess(ctx, db, ids) stamps users.tokens_valid_after with the current time truncated to whole seconds (soft-deleted rows included) and sets revoked_at on every not-yet-revoked user_api_tokens row of those users, leaving earlier revoked_at values unchanged; an empty list writes nothing. DeactivateUsers and BanUsers call it for every selected user (already deactivated or banned ones included) and return the same counts as before. + - offboarding_test.go, deactivate: a user with a session JWT issued at least one second before the action and a usr_ API token; after the admin deactivate bulk action the JWT is refused by GET /fetch and by POST /refresh (401, error "Could not refresh token"), the API token is refused by the APITokenGuard; a password login right after the deactivation (same second allowed) restores the account, sends the reactivate mail and returns a token that GET /fetch accepts; the old JWT and the old API token stay refused. + - offboarding_test.go, ban: after the ban bulk action the old JWT is refused by /fetch and /refresh and the API token by the guard; password login answers the invalid-login response; after unban the old JWT and API token stay refused and a fresh login works. + - offboarding_test.go, refresh subject checks without RevokeUserAccess: a user soft-deleted directly (deleted_at only) -> refresh 401; a user banned directly through a user_throttle row (no stamp) -> refresh 401; a token whose iat precedes tokens_valid_after set directly -> refresh 401; a suspended (not banned) user -> refresh 200; a normal user -> refresh 200 with last_seen stamped; a refused refresh stamps no last_seen. Existing TestRefresh, TestLogin (restore and reactivate mail) and TestDeactivatingEvent stay green. + - sociallogin TestInactiveAndBanned (matching_test.go:461) gains a banned-and-deactivated user: identity sign-in, verified email match and pending completion all answer the banned message, the account stays soft-deleted, no reactivate mail is sent and no identity row is created for it. TestSoftDeleted (restore of a deactivated, not banned account) stays unchanged and green. + + +User plugin, in ../sm-demo-app/plugins/golem15/user (master after Task 1, GOWORK=off): + +Add the exported RevokeUserAccess(ctx context.Context, db *gorm.DB, userIDs []uint) error to classes/admin_actions.go with the file's transaction convention (callers pass their transaction; use fresh(ctx, db)). One cutoff for both writes: time.Now() truncated to whole seconds, because JWT iat has whole-second precision and a token minted right after the revocation (a reactivating login) must be valid; a token minted earlier within that same second also stays valid, which the doc comment states. Writes: an unscoped UpdateColumn of users.tokens_valid_after for the ids, and an update of models.APIToken setting revoked_at where user_id is in the ids and revoked_at IS NULL. Doc comment: it ends every session JWT issued before the cutoff (the jwt guard, RequestUser and /refresh honour tokens_valid_after) and every personal API token of the users; applications may call it for their own offboarding, and after writing a ban directly to user_throttle. + +DeactivateUsers (:95-113): after the deleted_at writes call RevokeUserAccess for every non-nil selected user with an id, whether deactivated now or before (a repeated deactivate also cleans accounts deactivated before this change); keep the returned count. BanUsers (:140-168): call RevokeUserAccess for all ids after the throttle writes; keep the count. RestoreUsers and UnbanUsers stay unchanged: revoked tokens stay revoked. Update the doc comments of DeactivateUsers, BanUsers and DeactivatingEvent (classes/events.go:59-67: sessions and API tokens are already revoked when it fires). + +Refresh (2zy-4) in controllers/api_controller.go:178-215: keep the bouncer.Refresh call and its existing 401 body for token-level failures. After it succeeds, read the presented token's sub and iat with github.com/golang-jwt/jwt/v5 ParseUnverified (bouncer.Refresh already verified signature, audience, refresh window and blacklist), load the user with a scoped query by id through gdb and the request context, and refuse when the user is not found (missing or deactivated: a deactivated user signs in again, which reactivates the account), when TokensValidAfter is set and iat is before it, or when classes.ThrottleStates (with classes.ContextWithThrottle(ctx, app)) reports a ban. A refusal answers 401 with the existing shape: error "Could not refresh token", msg bouncer.ErrSubjectRejected.Error(); a database failure answers writeOpaque500. Suspension is deliberately not checked: it is the failed-password throttle, and refusing refresh on it would let anyone who knows an email log its owner out. Move TouchLastSeen after the checks so a refused refresh stamps nothing; replace the comment at :204-205 with one describing the subject check. + +Tests per the behavior block: unit cases in classes/admin_actions_test.go (its Postgres setup); offboarding_test.go in the plugin root with the admin harness for bulk actions (admin_harness_test.go) and the session helpers in session_test.go, plus the API-token guard path used by api_tokens_test.go; capture the reactivate mail the way session_test.go TestLogin does. + +User README.md: a short "Deactivation and bans" subsection under Features, and matching edits to the JWT sessions bullet (:20), Last seen (:33), Admin screens (:28) and the Bulk actions row (:129): deactivation is self-service and reversible (a password login or a golem15.sociallogin sign-in restores the account and sends the reactivate mail; an administrator can restore it too), and it ends every earlier session and personal API token, so the user signs in again and gets a fresh token, while old API tokens stay revoked; /refresh refuses deactivated and banned users and tokens from before tokens_valid_after; a ban is the lock: password login, social sign-in and refresh refuse it and its sessions and API tokens are revoked; unban and restore revive no token. Add RevokeUserAccess to the classes API list (:184) and an upgrade note: accounts deactivated or banned before this version keep their API tokens until deactivated or banned again (the actions are idempotent) or RevokeUserAccess runs for them. Update the Testing section. + +Commit in the user-plugin checkout: fix: deactivate and ban revoke sessions and API tokens, and /refresh refuses deactivated, banned and cut-off subjects (admin_actions.go, admin_actions_test.go, events.go, api_controller.go, offboarding_test.go, README.md). No push. + +Sociallogin, in ../sm-sociallogin-plugin (master, own go.work): no code change. Extend TestInactiveAndBanned with the banned-and-deactivated case per the behavior block (seed it like the banned user plus a soft delete; reuse the trash/restored helpers pattern from TestSoftDeleted). README.md: the matching table row (:49) keeps the restore and the line at :55 becomes: every sign-in requires an activated account without a ban; a deactivated account is restored by a social sign-in exactly as by a golem15.user password login (with the reactivate mail), a banned one is refused on every path and never restored. Add a Security notes bullet stating the same split (deactivation is reversible by its owner; a ban is the lock, checked before any restore or identity write). Update the Testing paragraph. + +Commit in ../sm-sociallogin-plugin: test: a banned account is refused and never restored by social sign-in, and the deactivation semantics are documented (matching_test.go, README.md). No push. + + + cd /media/nvme/dev/golem15/summercms.io/summercms/sm-demo-app/plugins/golem15/user && GOWORK=off go vet ./... && GOWORK=off go test -count=1 -run 'TestOffboarding|TestRefresh|TestLogin|TestDeactivatingEvent|TestAdminUserActions|TestRevokeUserAccess|TestSharedAPIToken|TestAPIToken' ./... && GOWORK=off go test -count=1 ./... && cd /media/nvme/dev/golem15/summercms.io/summercms/sm-sociallogin-plugin && go vet ./... && go test -count=1 -run 'TestSoftDeleted|TestInactiveAndBanned|TestMatching' ./... && go test -count=1 ./... + + Deactivate and ban revoke earlier session JWTs and personal API tokens; a deactivated user reactivates by signing in and gets a working token; a banned user is refused by login, social sign-in and refresh and is never restored; /refresh refuses deactivated, banned and cut-off subjects with the existing 401 body; both suites green; one commit in each repo; nothing pushed. + + + + Task 3: sm-sociallogin-plugin: Entra tenant ID in profile_data, SignedInEvent after every social sign-in, logged listener errors + ../sm-sociallogin-plugin/classes/provider.go, ../sm-sociallogin-plugin/classes/providers_goth.go, ../sm-sociallogin-plugin/classes/codes.go, ../sm-sociallogin-plugin/classes/flow.go, ../sm-sociallogin-plugin/classes/registration.go, ../sm-sociallogin-plugin/classes/events.go, ../sm-sociallogin-plugin/providers_goth_test.go, ../sm-sociallogin-plugin/fakeprovider_test.go, ../sm-sociallogin-plugin/signin_event_test.go, ../sm-sociallogin-plugin/README.md + + - TestGothProviders microsoft (providers_goth_test.go): the fake token answer carries an id_token whose payload has aud = the client id and tid = a GUID; Complete returns Profile.TenantID = that GUID in lower case. Without id_token, with a malformed id_token, with an aud that is not the client id, or with a tid that is not a GUID, TenantID is empty and the sign-in still succeeds. invalid_grant still maps to ErrCodeExpired; no request leaves the provider's hosts; the timeout case holds; the authorization URL cases are unchanged. + - signin_event_test.go (Postgres, fake provider with a TenantID): an identity sign-in fires one SignedInEvent with User, Provider "fake", ProviderID, IdentityID (the golem15 oauth identity row id), TenantID, Email (provider email), Action "login", Created false; a register-action creation fires it with Action "register", Created true; a link fires it with Action "link"; a pending completion fires it once; a sign-in that restores a deactivated account fires it once with Action "login". + - signin_event_test.go: profile_data holds tenant_id for a profile with a tenant and exactly name, email, avatar without one; a pending sign-in keeps the tenant through PendingPayload (tenant_id in the stored payload, omitted when empty) and the completed identity has profile_data.tenant_id. + - signin_event_test.go: a SignedInEvent listener returning an error does not fail the sign-in (token and user payload returned) and the error is logged; a listener that changes the user's name in the database is reflected in the returned user payload; a register/created listener returning an error does not fail social account creation and is logged. + + +All work in ../sm-sociallogin-plugin on master (after Task 2's commit). Keep the plugin's logging style (slog.Default(), as in controllers/*.go): sociallogin requires summercms v0.3.1, so do not use diary. + +Tenant ID (2zy-2, key tenant_id per OD-2). Add TenantID string to Profile (classes/provider.go:26-41): the directory tenant the account signed in from (Microsoft Entra tid), empty for providers without tenants. Add TenantID to PendingPayload (classes/codes.go:46-59) with JSON key tenant_id and omitempty; copy it in pendingOutcome (flow.go:233-252) and back in PendingPayload.Profile() (codes.go:63-77). In upsertIdentity (flow.go:385-422) add "tenant_id" to profile_data only when p.TenantID is not empty, so other providers keep exactly name, email, avatar (profile_data is json:"-" on the user plugin's OAuthIdentity, so no API response changes). + +Microsoft exchange (classes/providers_goth.go). goth's azureadv2 Session.Authorize drops the id_token, so the microsoft provider performs the code exchange itself and keeps goth for the authorization URL and the Graph profile. Give gothProvider an optional complete override (a func field taking ctx, the provider, the goth session string and the callback values, returning Profile and error); Complete (:72-82) delegates to it when set, so google, facebook and github are untouched. newMicrosoft (:225-254) builds an oauth2.Config identical to goth's: ClientID, ClientSecret, RedirectURL from cfg; AuthURL https://login.microsoftonline.com/{tenant}/oauth2/v2.0/authorize and TokenURL https://login.microsoftonline.com/{tenant}/oauth2/v2.0/token with the same tenant default (DefaultTenant); scopes from the same azureadv2 ScopeType constants converted with string(). The override exchanges the callback's code with context.WithValue(ctx, oauth2.HTTPClient, p.client); maps a failure through p.exchangeError (invalid_grant still becomes ErrCodeExpired, no response body in the error); refuses a token that is not Valid(); builds an azureadv2.Session with AccessToken, RefreshToken and ExpiresAt from the token; calls p.gp.FetchUser with it and passes the result through the existing microsoft profile mapping; then sets TenantID from token.Extra("id_token"). Add a helper (for example entraTenantID(idToken, clientID string) string) that splits the compact JWT into three parts, base64url-decodes the payload (raw URL encoding, tolerating padding), reads the string claims tid and aud, and returns the lower-cased tid only when aud equals clientID and tid is a canonical GUID (8-4-4-4-12 hex), otherwise the empty string. The signature is not verified: the id_token comes straight from the token endpoint over TLS in a client-authenticated back-channel exchange (OpenID Connect Core 3.1.3.7); write that reason in the helper's doc comment. Never log the id_token, the access token or the code. Update newMicrosoft's doc comment. + +SignedInEvent (2zy-3). Create classes/events.go with SignedInEventName = "sociallogin.SignedIn" and type SignedInEvent struct with User (*usermodels.User), Provider (registry name), ProviderID (the identity's provider_id), IdentityID (uint, the golem15 oauth identity row id), TenantID, Email (provider-reported, possibly empty), Action (ActionLogin, ActionRegister or ActionLink) and Created (true when this sign-in created the account); doc comment: fired after every successful social sign-in, after the identity is stored and FinishLogin ran (so a deactivated account is already restored), before the session token is minted, outside any transaction; listener errors are logged and never fail the sign-in; the user row is re-read afterwards so listener writes reach the token payload. Make upsertIdentity return the identity row id with the error. Change signIn (flow.go:311-324) to receive the provider, the profile, the identity id and the created flag: after FinishLogin it fires the event through app.Events (skipped when app or app.Events is nil), logs a listener error with slog.Default().Error("golem15.sociallogin: sign-in listener failed", "provider", ..., "user_id", ..., "err", ...), then re-reads the user row by id (scoped; any read failure, including a row a listener removed, is returned as an internal error), then IssueToken and APIArray as now. loginWithIdentity (:287-307) and createUser (registration.go:112-185; Created true, Action register) pass the values through; linkIdentity (:128-164) fires the event with Action link and Created false after its transaction and re-reads the user the same way before IssueToken. In createUser replace the discarded FireRegisterEvent / FireCreatedEvent results (registration.go:181-183) with the same Error-level logging ("register listener failed", "created listener failed") and keep the comment that a committed registration is never undone. + +Tests per the behavior block: extend the microsoft cases in providers_goth_test.go (build the unsigned id_token in the test from base64url-encoded JSON header and payload plus any signature segment; the fake token bodies at :128-130 and :181 gain an id_token field); extend fakeprovider_test.go so a fake profile can carry a TenantID; add signin_event_test.go with a listener on the app's event bus registered the way existing tests do (search this repo's and the user plugin's tests for event listener registration) and a slog handler swap to assert the logged failure. + +README.md: Features gains the Microsoft tenant line (profile_data.tenant_id from the id_token tid claim, only when known; other providers unchanged) and an Events subsection documenting SignedInEvent / SignedInEventName with every field, when it fires (login, register, link, pending completion, including a restoring sign-in), Created, the re-read, and that listener errors (and user.Register / user.Created listener errors during social registration) are logged and never fail the sign-in; the Account matching section notes that the pending payload keeps the tenant; Security notes gain the tid trust basis (back-channel id_token, aud must equal the client id, GUID format, signature not checked and why) and the advice to match TenantID together with Provider "microsoft" when assigning organisations by tenant; the API reference adds SignedInEvent, SignedInEventName, Profile.TenantID and PendingPayload.TenantID; the Testing paragraph lists the new tests. + +Commits in ../sm-sociallogin-plugin, explicit paths, no co-author tags: (a) feat: record the Entra tenant ID of a Microsoft sign-in in profile_data (provider.go, providers_goth.go, codes.go, flow.go tenant parts, providers_goth_test.go, fakeprovider_test.go, README); keep tenant assertions that need the event out of this commit; (b) feat: fire SignedInEvent after every social sign-in and log listener failures (events.go, flow.go, registration.go, signin_event_test.go, README). No push. + +Cross-check both plugins together (no repo is modified): write a go.work in your scratchpad directory with go 1.27.0, a use directive with the absolute path of ../sm-sociallogin-plugin, and replace directives sending git.golem15.com/golem15/summercms to the absolute path of summercms.go and git.golem15.com/golem15/sm-user-plugin to the absolute path of ../sm-demo-app/plugins/golem15/user; run the sociallogin suite with GOWORK set to that file and report the result in the SUMMARY. + + + cd /media/nvme/dev/golem15/summercms.io/summercms/sm-sociallogin-plugin && go vet ./... && go test -count=1 -run 'TestGothProviders|TestSignedInEvent|TestSoftDeleted|TestInactiveAndBanned|TestMatching|TestRegisterComplete|TestLink|TestErrorMessages' ./... && go test -count=1 ./... && grep -c 'tenant_id' README.md + + Microsoft sign-ins store profile_data.tenant_id from a validated tid; SignedInEvent fires once per successful login, register, link and pending completion with every field; listener errors are logged and never fail a sign-in; the sociallogin suite is green against the published user plugin (its go.work) and against the Task 1-2 user plugin (scratch GOWORK); two commits; nothing pushed. + + + + Task 4: cabana ClearOnChange (summercms.go) and the user plugin's stale-role fix on top of it + modules/cabana/relation.go, modules/cabana/relation_field.go, modules/cabana/crud.go, modules/cabana/clear_on_change_test.go, modules/cabana/README.md, docs/backend/relation-manager.md, ../sm-demo-app/plugins/golem15/user/controllers/organisations_admin_controller.go, ../sm-demo-app/plugins/golem15/user/controllers/users_admin_controller.go, ../sm-demo-app/plugins/golem15/user/admin_organisations_edge_test.go, ../sm-demo-app/plugins/golem15/user/README.md + Phase 19.8 has released (summercms.go tag v0.4.2 exists and 19.8-06 is complete), or the user has explicitly allowed this cabana commit into the 19.8 release range; and git status --porcelain -- modules/cabana modules/pact docs/backend in summercms.go is empty (no other session editing those paths). + + - clear_on_change_test.go, hasMany (Postgres, the relation_child_test.go fixtures): a RelationContract with ClearOnChange naming a nullable child column; link sets the foreign key and clears that column on the linked child; unlink clears the foreign key and the column; a contract without ClearOnChange leaves the column untouched (existing behaviour); a deferred link committed by the parent's first save clears it too. + - clear_on_change_test.go, belongsTo form field: a FieldRelationContract with ClearOnChange; an update that changes the key (to another record, and to empty) clears the column; an update that keeps the key leaves it; an update that changes the key and also submits the column through its own form field keeps the submitted value; a create that sets the key and does not submit the column clears it. + - clear_on_change_test.go, boot: ClearOnChange on a belongsToMany field contract or a belongsToMany relation contract, a column the model does not have, a non-pointer (not nullable) column, the ForeignKey itself, or the primary key each stop the start-up with an error naming the controller and the column. + - user plugin admin_organisations_edge_test.go:183-213 flipped and renamed TestAdminOrganisationMembershipClearsRole: after the members-manager unlink both organisation_id and organisation_role are NULL; after linking into the second organisation the role stays NULL; an organisation-less user with a leftover role, linked through the members manager, ends with role NULL; changing a user's organisation in the Users form to another organisation, and to none, clears the role; a Users form save that keeps the organisation keeps the role; deleting an organisation still clears both (existing part). + + +Framework, in summercms.go (OD-3). Add ClearOnChange []string to cabana.RelationContract (modules/cabana/relation.go:38-58) and to cabana.FieldRelationContract (modules/cabana/relation_field.go:22-45), each with a doc comment: nullable columns of the related child (hasMany) or of the record (belongsTo) set to NULL whenever this relation changes the foreign key, so values that only make sense for one parent (a role in an organisation, for example) never move to another or outlive the membership; the zero value changes nothing. + +Validation at boot: in validateHasManyContract / validateRelationContract (relation.go:479 and :558) and in compileFieldRelation (relation_field.go:205), refuse ClearOnChange on a belongsToMany contract, and refuse any listed column that the model does not have (fieldByColumn over modelFields), whose field is not a pointer, or that is the contract's ForeignKey or the model's primary key; the error names the controller and the column, in the style of the existing contract errors. + +Application: in linkRelated's hasMany branch (relation.go:917-975, before the Save at :955) and in unlinkRelated's hasMany branch (:1231-1260, before the Save at :1250), after setModelColumn sets the foreign key, set each ClearOnChange column of the child to nil with setModelColumn (link and unlink always change the key: link adopts only children whose key is NULL, unlink only children of this parent). In assignBelongsTo (relation_field.go:648-663), before setting the foreign key, read the current key value of the model (fieldByColumn; nil pointer or zero counts as empty) and compare it with the submitted id (or empty); when they differ, set each ClearOnChange column to nil unless the same save writes that column through its own form field. To know that, assignBelongsTo receives the set of fill keys this save wrote: the keys of fillAllowed(cc, target, op) that are present in projected, computed in the save function in crud.go where both are in scope; pass it from the single assignBelongsTo call site in crud.go. No other behaviour changes; the deferred unlink of a not-yet-saved parent writes no child row and needs nothing. + +Tests: modules/cabana/clear_on_change_test.go per the behavior block, reusing the Postgres fixtures and helpers of relation_child_test.go / phase122_fixture_test.go and the form-save helpers of phase121_form_test.go (its TestWritableForeignKeySmoke is the closest model); boot cases next to the style of phase121_schema_boot_test.go. Docker-backed parts skip under -short like their neighbours. + +Docs in the same commit (CLAUDE.md): modules/cabana/README.md, the relations sentence in Features (around :28) and the API reference rows for FieldRelationContract and RelationContract (around :272 and :275) mention ClearOnChange; docs/backend/relation-manager.md gets a short section after the WritableForeignKey paragraph (around :52) for belongsTo fields and a sentence in the hasMany part (around :117-133 and the link/unlink description at :190), with an organisation membership and role as the example and the "unless the same save writes it" rule. If you add a code example through an src= snippet, put it in an example_*_test.go and run summer docs:sync. Run go run ./cmd/summer docs:build --check and go test -run TestDocsTree ./cmd/summer. + +Commit in summercms.go, explicit paths only (other sessions have uncommitted files there; leave them alone), no co-author tags: feat(cabana): ClearOnChange clears dependent columns when a relation changes a foreign key (relation.go, relation_field.go, crud.go, clear_on_change_test.go, README.md, docs/backend/relation-manager.md). go vet ./... and go test -short ./... green (and go test ./modules/cabana/... with Docker); if a failure comes only from another session's uncommitted files, record it in the SUMMARY and do not touch those files. Do not push or tag. + +User plugin, in ../sm-demo-app/plugins/golem15/user (master after Task 2, GOWORK=off; it builds against summercms.go's working tree through the go.mod replace, so it sees the commit above). Set ClearOnChange: organisation_role on the members RelationContract (organisations_admin_controller.go:76-90) and on the organisation FieldRelationContract (users_admin_controller.go:370-388). Update the doc comments at organisations_admin_controller.go:21-27 ("sets and clears users.organisation_id and nothing else") and :73-75, and at users_admin_controller.go:364-369: adding or removing a member, and changing the organisation in the user form, clear organisation_role unless the same save writes it. Flip and rename the known-gap test and add the user-form cases per the behavior block (adminEnv harness: newAdminEnv, admin, call/expect, seedUser, reload; find the users update route in admin_users_test.go), and replace the test's doc comment (12.1-SECURITY-REVIEW open item 6 is closed). README.md: the Organisations bullet (:26) and the Organisations screen bullet (:30) state the role rule; add a Dependencies note that this behaviour needs a summercms version with cabana ClearOnChange (the tag decided at Task 5); update the Testing section. Leave go.mod's summercms require as it is in this commit: the require bump to the released tag is a separate commit at Task 5, once the tag exists. + +Commit in the user-plugin checkout: fix: clear a stale organisation_role when a user's organisation changes in the admin (organisations_admin_controller.go, users_admin_controller.go, admin_organisations_edge_test.go, README.md). This must be the last commit on the user plugin's master, so the earlier fixes can be tagged without it. No push. + + + cd /media/nvme/dev/golem15/summercms.io/summercms/summercms.go && go vet ./... && go test -count=1 -run 'TestClearOnChange|TestRelationChild|TestWritableForeignKey' ./modules/cabana/... && go test -count=1 -short ./... && go test -count=1 -run '^TestDocsTree$' ./cmd/summer && go run ./cmd/summer docs:build --check && cd /media/nvme/dev/golem15/summercms.io/summercms/sm-demo-app/plugins/golem15/user && GOWORK=off go vet ./... && GOWORK=off go test -count=1 -run 'TestAdminOrganisation|TestAdminUser' ./... && GOWORK=off go test -count=1 ./... + + cabana.RelationContract and cabana.FieldRelationContract accept ClearOnChange with boot validation, documented in the cabana README and docs/backend/relation-manager.md, tested on Postgres; the user plugin clears organisation_role on member add/remove and on a user-form organisation change; the known-gap test asserts the closed gap; framework short suite, docs checker and user-plugin suite green; one commit in summercms.go and one (last) commit in the user plugin; nothing pushed or tagged. + + + + Task 5: Go-ahead for push, tags, changelog and submodule pointer bumps + Whether and how far to publish: the summercms.go cabana commit, the user-plugin commits (two releasable sets), the sociallogin commits, and the submodule pointers in the apps. + + Local commits after Tasks 1-4. sm-user-plugin master (../sm-demo-app/plugins/golem15/user): Task 1 (two commits) and Task 2 (one) need only summercms v0.4.0; Task 4's commit, the last one, needs a summercms tag containing ClearOnChange. sm-sociallogin-plugin master: Task 2 (one) and Task 3 (two); go.mod unchanged (still requires sm-user-plugin v0.1.5 and summercms v0.3.1). summercms.go master: one cabana commit after the 19.8 release. Review with git log --oneline origin/master..master in each repo. + Publishing steps for publish-all, in this order (CLAUDE.md: framework and plugins first, then each app's pointer bump as its own commit; ssu:status / ssu:push can drive the submodule parts): + (1) summercms.go: git fetch, confirm origin/master..master holds the cabana commit (plus planning docs), vet, short suite and docs checker green, push master, annotated tag v0.4.3 (proposed: the tag after 19.8's v0.4.2), push the tag, and the same-day changelog post from ../sm-summercmsio-app with scripts/changelog-post.py (source, then publish with user-facing notes; never print the token). + (2) sm-user-plugin: tag v0.1.7 at the Task 2 commit (registration allow-list, listener logging, offboarding; compatible with summercms v0.4.0); then a commit "chore: require summercms v0.4.3" (go mod edit of the require, then GOWORK=off go mod tidy once the tag is fetchable, tests green), tag v0.1.8 there (adds the stale-role fix); push master and both tags. + (3) sm-sociallogin-plugin: push master, tag v0.1.3, push the tag. + (4) Apps, one commit per app and plugin, staging only the pointer and go.mod/go.sum: the user plugin in fonoteka.go, sm-bm-app, sm-demo-app, sm-grzybyfunkcjonalne-app and sm-summercmsio-app; sociallogin in sm-bm-app and sm-demo-app. An app moving to user v0.1.8 also needs summercms v0.4.3 or newer (bump its summercms require where it pins one); an app that cannot move its framework yet takes v0.1.7. Run each app's go build ./... before committing; leave other sessions' edits alone; push each app. + + + + + + + Select: publish-all (optionally naming which apps), plugins-first, or hold. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| anonymous client -> POST /_user/api/v1/register | untrusted JSON body is mass-assigned into the users row | +| session JWT holder -> POST /_user/api/v1/refresh | a token of a user who has since been deactivated, banned or cut off | +| personal API token holder -> plugin APIs (APITokenGuard) | long-lived secret of a user who has since been deactivated or banned | +| OAuth provider (Microsoft token endpoint, Graph) -> sociallogin | identity, email and tenant asserted by the provider | +| administrator -> cabana Organisations / Users screens | membership changes that must not carry privileges across organisations | +| sociallogin -> application listeners | plugin code reacting to sign-ins; failures must not change the sign-in outcome | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan | +|-----------|----------|-----------|----------|-------------|-----------------| +| T-2zy-01 | E | /register via RegisterUser mass assignment (organisation fields, is_onboarded, must_change_password) | critical | mitigate | Task 1: explicit RegisterFillable allow-list; TestRegisterIgnoresProtectedFields over HTTP and RegisterUser | +| T-2zy-02 | S | created_ip_address / last_ip_address from the body when RegisterOptions.IP is empty | low | mitigate | Task 1: IP columns set only from RegisterOptions.IP after the fill | +| T-2zy-03 | E | organisation_role kept after a membership change (members manager, Users form) | high | mitigate | Task 4: cabana ClearOnChange on both contracts; flipped known-gap test plus form cases | +| T-2zy-04 | E | controllers.Refresh sliding sessions of deactivated, banned or cut-off users | high | mitigate | Task 2: subject check after bouncer.Refresh (scoped load, tokens_valid_after, ThrottleStates ban); offboarding_test.go | +| T-2zy-05 | E | session JWTs and user_api_tokens surviving deactivate/ban and reviving on restore/unban | high | mitigate | Task 2: RevokeUserAccess from DeactivateUsers and BanUsers; restore/unban revive nothing | +| T-2zy-06 | E | a banned account restored or signed in socially | high | mitigate | Existing checkCanSignIn ban check runs before FinishLogin; Task 2 pins it for banned-and-deactivated accounts on every path | +| T-2zy-07 | S | Entra tid read from an id_token without signature verification | medium | mitigate | Task 3: only from the back-channel token response over TLS, aud must equal the client id, GUID format, empty otherwise; README trust note; apps pair TenantID with Provider microsoft | +| T-2zy-08 | I | new log lines | medium | mitigate | Tasks 1 and 3: log only provider, user_id and err; never id_token, access token, code or secrets (TestSettings no-secret-in-logs stays green) | +| T-2zy-09 | R | discarded user.Register / user.Created / sign-in listener errors | low | mitigate | Tasks 1 and 3: Error-level log lines with user_id; control flow unchanged | +| T-2zy-10 | D | suspension (failed-password throttle) used as a refresh refusal | medium | accept | Refresh checks bans only, so failing logins against an email cannot log its owner out | +| T-2zy-11 | E | deactivated account reactivated by its owner (password or social sign-in) | low | accept | User decision OD-1: deactivation is self-service and reversible; a ban is the lock and is refused everywhere | +| T-2zy-12 | E | a JWT minted within the same second before a revocation stays valid until it expires | low | accept | Cutoff truncated to whole seconds (JWT iat precision) so a reactivating login in that second works; refresh and login still refuse bans | +| T-2zy-SC | T | npm/pip/cargo/go installs | low | accept | No new dependency: golang-jwt/jwt/v5 (user plugin), golang.org/x/oauth2 and goth (sociallogin) are already direct requirements; cabana uses only existing code | + + + +- User plugin (../sm-demo-app/plugins/golem15/user): GOWORK=off go vet ./... and GOWORK=off go test -count=1 ./... green with Docker after every task. +- Sociallogin (../sm-sociallogin-plugin): go vet ./... and go test -count=1 ./... green with its own go.work, and the scratch-GOWORK run against the modified user plugin green. +- summercms.go after Task 4: go vet ./..., go test -short ./..., go test ./modules/cabana/... (Docker), TestDocsTree and summer docs:build --check green; only the Task 4 paths staged. +- No push, tag, changelog post or app commit before the Task 5 decision. + + + +- 2zy-1: a /register body sets only name, surname, email, password, preferred_locale and marketing_consent; IP columns are server-set; admin and application flows unchanged. +- 2zy-2: Microsoft identities carry profile_data.tenant_id; SignedInEvent carries TenantID. +- 2zy-3: SignedInEvent fires on every successful social sign-in with provider and identity; register, created and sign-in listener errors are logged. +- 2zy-4: deactivation ends sessions and API tokens and is undone only by the owner signing in again (fresh token) or an administrator; a ban is refused by login, social sign-in and refresh and revokes sessions and API tokens; /refresh refuses deactivated, banned and cut-off subjects. +- 2zy-5: cabana ClearOnChange exists and is documented; adding or removing a member and changing the organisation in the Users form clear organisation_role; the known-gap test asserts the closed gap. +- Both plugin READMEs document the allow-list, the new event, the profile_data key, the refresh rule and the deactivation/ban semantics in the commits that change them; the cabana README and docs/backend/relation-manager.md document ClearOnChange in the framework commit. + + + +Create `.planning/quick/261022-2zy-user-and-sociallogin-plugin-security-gap/261022-2zy-SUMMARY.md` when done, listing every commit hash per repo (summercms.go, sm-user-plugin, sm-sociallogin-plugin), the scratch-GOWORK cross-check result, anything skipped because another session had the files open, and the Task 5 outcome. + diff --git a/.planning/quick/261022-2zy-user-and-sociallogin-plugin-security-gap/261022-2zy-SUMMARY.md b/.planning/quick/261022-2zy-user-and-sociallogin-plugin-security-gap/261022-2zy-SUMMARY.md new file mode 100644 index 00000000..0f749dfa --- /dev/null +++ b/.planning/quick/261022-2zy-user-and-sociallogin-plugin-security-gap/261022-2zy-SUMMARY.md @@ -0,0 +1,149 @@ +--- +phase: quick-261022-2zy +plan: 01 +quick_id: 261022-2zy +subsystem: sm-user-plugin, sm-sociallogin-plugin, cabana +tags: [security, registration, offboarding, entra, events, cabana, relations] +status: complete +requires: [summercms v0.4.2 released] +provides: + - User.RegisterFillable (registration allow-list) + - classes.RevokeUserAccess; deactivate/ban revoke sessions and API tokens; /refresh subject check + - sociallogin Profile.TenantID, PendingPayload.TenantID, profile_data.tenant_id + - sociallogin classes.SignedInEvent / SignedInEventName + - cabana RelationContract.ClearOnChange and FieldRelationContract.ClearOnChange +affects: [fonoteka.go, sm-bm-app, sm-demo-app, sm-grzybyfunkcjonalne-app, sm-summercmsio-app] +tech-stack: + added: [] + patterns: [explicit registration allow-list, revocation cutoff in whole seconds, declarative ClearOnChange on relation contracts] +key-files: + created: + - ../sm-demo-app/plugins/golem15/user/offboarding_test.go + - ../sm-sociallogin-plugin/classes/events.go + - ../sm-sociallogin-plugin/signin_event_test.go + - ../sm-sociallogin-plugin/tenant_test.go + - modules/cabana/clear_on_change_test.go + - modules/cabana/clear_on_change_field_test.go + modified: + - ../sm-demo-app/plugins/golem15/user/models/user.go + - ../sm-demo-app/plugins/golem15/user/controllers/registration.go + - ../sm-demo-app/plugins/golem15/user/controllers/api_controller.go + - ../sm-demo-app/plugins/golem15/user/classes/admin_actions.go + - ../sm-demo-app/plugins/golem15/user/controllers/organisations_admin_controller.go + - ../sm-demo-app/plugins/golem15/user/controllers/users_admin_controller.go + - ../sm-sociallogin-plugin/classes/flow.go + - ../sm-sociallogin-plugin/classes/providers_goth.go + - ../sm-sociallogin-plugin/classes/registration.go + - modules/cabana/relation.go + - modules/cabana/relation_field.go + - modules/cabana/crud.go +decisions: + - "Registration fills only User.RegisterFillable (name, surname, email, password, preferred_locale, marketing_consent); IP columns only from RegisterOptions.IP; Fillable() unchanged" + - "RevokeUserAccess cutoff is time.Now() truncated to whole seconds (JWT iat precision)" + - "/refresh refuses missing/deactivated, banned and pre-tokens_valid_after subjects with the existing 401 body; suspension is not checked" + - "Microsoft provider exchanges the code itself to keep the id_token; tid trusted only with aud == client id and GUID format" + - "cabana ClearOnChange is applied only on hasMany link/unlink and on a belongsTo key change in the form save; created-child commits are untouched" +metrics: + duration: ~95 min + completed: 2026-10-22 +actuals: + tokens: 46000 + tasks: 4 + commits: 1 +plan_head_before: 9563bc8 +plan_head_after: 9516a2b +--- + +# Quick 261022-2zy: user and sociallogin plugin security gaps Summary + +Registration allow-list, session and API-token revocation on deactivate/ban with a /refresh subject check, Entra tenant ID and a SignedInEvent in sociallogin, and an additive cabana `ClearOnChange` that clears a stale `organisation_role`. Tasks 1 to 4 are committed locally in three repos. Task 5 (push, tags, changelog, pointer bumps) is waiting for the user's go-ahead. + +`commits`/`plan_head_*` in the frontmatter measure summercms.go only (one framework commit since 9563bc8). The plugin repos are listed below. + +## Commits + +### sm-user-plugin (`../sm-demo-app/plugins/golem15/user`, branch master, base 9986342 = origin/master = v0.1.6) + +| Hash | Subject | +|------|---------| +| 499b6dc | test: migration rollback counts include the email_verified_at migration (deviation, see below) | +| b6b7ac7 | fix: registration fills only an explicit allow-list and never takes the IP columns from the body | +| e0b5ffe | fix: log /register listener failures instead of discarding them | +| c3712dc | fix: deactivate and ban revoke sessions and API tokens, and /refresh refuses deactivated, banned and cut-off subjects | +| aa83969 | fix: clear a stale organisation_role when a user's organisation changes in the admin (last commit; needs summercms with ClearOnChange) | + +### sm-sociallogin-plugin (`../sm-sociallogin-plugin`, master, base 9fd58db = origin/master) + +| Hash | Subject | +|------|---------| +| 04f4583 | test: a banned account is refused and never restored by social sign-in, and the deactivation semantics are documented | +| 5c9f846 | feat: record the Entra tenant ID of a Microsoft sign-in in profile_data | +| 4d3b2dd | feat: fire SignedInEvent after every social sign-in and log listener failures | + +go.mod unchanged (still requires sm-user-plugin v0.1.5 and summercms v0.3.1). + +### summercms.go (master, after v0.4.2) + +| Hash | Subject | +|------|---------| +| 9516a2b | feat(cabana): ClearOnChange clears dependent columns when a relation changes a foreign key | + +Nothing pushed or tagged. The sm-demo-app working tree now shows `M plugins/golem15/user`, the checkout moved to aa83969. That pointer bump is not committed; it belongs to Task 5. + +## Test results + +- User plugin: `GOWORK=off go vet ./...` and `GOWORK=off go test -count=1 ./...` (Docker) green before every commit, the last one included. +- Sociallogin: `go vet ./...` and `go test -count=1 ./...` green with its own go.work, which uses the published user plugin at 9986342. +- Scratch-GOWORK cross-check: sociallogin against summercms.go's working tree and the modified user plugin (aa83969), using `/tmp/claude-1000/.../scratchpad/go.work`. Green: `ok git.golem15.com/golem15/sm-sociallogin-plugin`. +- summercms.go: `go vet ./...`, `go test -count=1 -short ./...`, `go test -count=1 ./modules/cabana/...` (Docker, 82 s), `go test -run '^TestDocsTree$' ./cmd/summer` and `go run ./cmd/summer docs:build --check` all green. + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 3 - Blocking] Pre-existing migration rollback test failures in the user plugin** +- **Found during:** Task 1, the first full suite run. +- **Issue:** `updates/` TestUserGroupsMigration, TestExtendUsersMigration, TestCreateUserThrottleMigration and TestCreateJwtBlacklistMigration failed at origin/master. Their rollback counts were one short since migration 202610210001 (email_verified_at, v0.1.6) was added. This blocked "full suite green at every commit". +- **Fix:** Bumped the four rollback counts and their comments. Test-only change, committed separately. +- **Commit:** 499b6dc + +**2. [Rule 2] Extra test for the /register listener-error logging** +- Added TestRegisterListenerErrors (register_test.go) to commit e0b5ffe. The plan had no test for that commit. + +**3. [Rule 1] TestAdminOrganisationMembers pinned the old behaviour** +- admin_organisations_test.go expected a foreign member's role to survive unlink and link. It now expects the unlink to clear the role. Committed with aa83969. + +**4. Test file layout (sociallogin)** +- The tenant tests that do not need the event live in a new `tenant_test.go` (TestTenantProfileData) inside commit 5c9f846. The plan kept event-dependent assertions out of that commit; this keeps the tenant behaviour tested in the same commit that adds it. +- `fakeprovider_test.go` was not changed: the fake provider already returns whatever `classes.Profile` it is given, so the new `TenantID` field passes through. +- To capture mails, TestInactiveAndBanned now boots with the `log` mail driver and an slog handler. A deactivated-without-ban control subtest proves the capture sees the reactivate mail. + +**5. Test file layout (cabana)** +- The belongsTo cases are in an internal-package file, `clear_on_change_field_test.go` (TestClearOnChangeFieldBoot, TestClearOnChangeBelongsTo). They use a small dedicated fixture because no roster column is both nullable and fillable. +- The hasMany cases are in `clear_on_change_test.go` (TestClearOnChangeRelationBoot, TestClearOnChangeHasMany), on the df fixtures. +- The docs checker refuses Go code blocks without `src=`. So the docs example is the existing `example_form_seams_test.go` MembersController team contract, which gained `ClearOnChange: []string{"team_role"}` and a `Member.TeamRole` field; `summer docs:sync` refreshed the snippet. Its Example output is unchanged. +- Boot errors name the relation or field and the column, and `bootErr` adds the plugin and controller, as the existing contract errors do. + +**6. ClearOnChange scope** +- Applied in linkRelated and unlinkRelated for hasMany, which covers deferred link and unlink because they use the same path, and in assignBelongsTo. +- Not applied when a deferred commit attaches a child created in the form session, or when CreateChild sets the key: that would wipe values the admin just entered in the child form ("no other behaviour changes"). + +**7. User README dependency note** +- It says the role cleanup needs "SummerCMS v0.4.3 or later". If Task 5 picks a different tag, update this line together with the go.mod require bump. + +## Known Stubs + +None. + +## Threat Flags + +None beyond the plan's threat model. The new log lines carry only provider, user_id and err; the id_token, access token and code are never logged. + +## Task 5: awaiting go-ahead + +Blocking checkpoint:decision. Choose publish-all, plugins-first or hold. Nothing has been pushed, tagged, posted or pointer-bumped. + +## Self-Check: PASSED + +- Files exist: offboarding_test.go, classes/events.go (sociallogin), signin_event_test.go, tenant_test.go, modules/cabana/clear_on_change_test.go, modules/cabana/clear_on_change_field_test.go. +- Commits exist: 499b6dc, b6b7ac7, e0b5ffe, c3712dc, aa83969 (user plugin); 04f4583, 5c9f846, 4d3b2dd (sociallogin); 9516a2b (summercms.go).