fix: registration fills only an explicit allow-list and never takes the IP columns from the body

User.RegisterFillable lists what a registration body may set: name,
surname, email, password, preferred_locale and marketing_consent.
RegisterUser fills from it instead of Fillable, so organisation_id,
organisation_role, is_onboarded and must_change_password can no longer be
mass-assigned through POST /register, and created_ip_address and
last_ip_address come only from RegisterOptions.IP. Fillable is unchanged
for the admin form and trusted application code.

README.md

@@ -16,7 +16,7 @@ It owns the `users` table extensions, `user_throttle`, `jwt_blacklist`, `user_ap
 
 ## Features
 
-- **Registration and activation.** `POST /register`, `POST /activate`, `POST /activate-by-code`. Registration can be switched off and throttled; `activate_mode` is `auto`, `user` (a mailed code) or `admin`, with codes from `IssueActivationCode` and `VerifyActivationCode`.
+- **Registration and activation.** `POST /register`, `POST /activate`, `POST /activate-by-code`. Registration can be switched off and throttled; `activate_mode` is `auto`, `user` (a mailed code) or `admin`, with codes from `IssueActivationCode` and `VerifyActivationCode`. A `/register` body sets only the `User.RegisterFillable` columns: `name`, `surname`, `email`, `password` (stored as its hash), `preferred_locale` and `marketing_consent`. It cannot set `organisation_id` or `organisation_role` (membership is the administrator's or the application's), `is_onboarded`, `must_change_password`, or the activation, reset, address proof, handle and permission columns; other keys are ignored (they still reach `RegisterEvent.Payload`). `created_ip_address` and `last_ip_address` are server-set from the client address only.
 - **JWT sessions.** `POST /login`, `GET /fetch`, `POST /refresh`, `POST /logout`. Boot registers the `jwt` guard with the framework's `bouncer.Registry` and exposes it as the `jwt.auth` middleware for other plugins. Logged-out tokens go to the Postgres `jwt_blacklist`, which a background sweep prunes.
 - **Password reset and change.** `POST /forgot-password` answers the same body for known and unknown addresses; `POST /reset-password` consumes a code from `IssueResetCode`. `POST /change-password` requires the current password.
 - **Must-change-password lock.** The `user:require-password-change` console command flags an account. The flag rides on the authenticated principal (`bouncer.Principal.MustChangePassword`) for guards and other plugins to act on, and a successful `POST /change-password` clears it.
@@ -168,7 +168,7 @@ Migrations added for address proof, all additive:
 Packages:
 
 - `controllers`: the route handlers above, `AdminControllers` (the admin controller list), `PermissionAccessUsers` and `PermissionAccessGroups`, plus the registration steps `POST /register` runs, for other plugins:
-  - `RegisterUser(ctx, app, db, fields, RegisterOptions)` validates `fields` against the user model's rules, hashes the password with `golem15.user.password.bcrypt_cost` and creates the user on `db` (which may be a transaction). It returns the validation errors instead of writing a response and never modifies `fields`. `RegisterOptions.Activate` creates the account activated; `RegisterOptions.IP` records the client IP; `RegisterOptions.PasswordLess` stores `has_self_set_password=false` for an account whose random password the user never knew (a social sign-in), while the zero value keeps `true`.
+  - `RegisterUser(ctx, app, db, fields, RegisterOptions)` validates `fields` against the user model's rules, hashes the password with `golem15.user.password.bcrypt_cost` and creates the user on `db` (which may be a transaction). It fills only the `User.RegisterFillable` columns and ignores every other key of `fields`: an organisation membership and role are set with an explicit update after creation, as an application's onboarding does. It returns the validation errors instead of writing a response and never modifies `fields`. `RegisterOptions.Activate` creates the account activated; `RegisterOptions.IP` records the client IP and is the only source of `created_ip_address` and `last_ip_address` (both stay NULL without it, whatever `fields` says); `RegisterOptions.PasswordLess` stores `has_self_set_password=false` for an account whose random password the user never knew (a social sign-in), while the zero value keeps `true`.
   - `FireRegisterEvent(ctx, app, user, fields)` fires `RegisterEvent` with `Payload` set to a copy of `fields` without `password` and `password_confirmation`, and returns the listeners' error (`/register` ignores it). Fire it after the user row is committed.
   - `FireCreatedEvent(ctx, app, user)` fires `CreatedEvent` (`user.Created`). `/register` ignores that error.
   - `FireDeletingEvent(ctx, app, user, tx)` fires `DeletingEvent` (`user.Deleting`) inside the delete transaction.
@@ -188,7 +188,7 @@ Packages:
   - `AvatarURL(ctx, app, user, size)`, the display picture rule of the user payload at any size: the avatar thumbnail (size by size, mode auto) when the user has an avatar and a file bucket is published, the file path when the thumbnail fails, otherwise the gravatar URL. The URL may be protocol-relative or relative. The payload's `avatar_url` comes from it at 128.
   - `HandleGenerator`, `AssignHandle(ctx, db, user)`, `CreatedEvent` / `CreatedEventName`, `DeletingEvent` / `DeletingEventName`, `DeactivatingEvent` / `DeactivatingEventName`, `AuthorizeUser` and `RegisterUserAuthorizer`.
 - `middleware`: `APITokenGuard`, `Scope`, `Permission` and `BearerToken`.
-- `models`: `User`, `Throttle`, `Organisation`, `UserGroup`, `UsersGroup` (the `users_groups` pivot), `APIToken`, `OAuthIdentity`, `FrontendPermission` and `PermissionSet`. `PermissionSet` reads a stored permission column tolerantly (NULL, an empty string, `[]`, numeric strings) and writes a JSON object of integers; `ParsePermissionSet` and `PermissionSet.Text` do the same for a column that is a plain string on its model. `UserGroup.UsersCount` is filled only by a query that selects `users_count` (the admin list does). `Organisation.Members` is the relation behind the members manager, `Organisation.AttachRelations` declares its avatar (`OrganisationAvatarField`), and `Slugify` is the slug rule. `User.FilterScope` answers the admin list scope `FilterByGroup`, and `User.AttachRelations` declares the avatar (`AvatarField`).
+- `models`: `User`, `Throttle`, `Organisation`, `UserGroup`, `UsersGroup` (the `users_groups` pivot), `APIToken`, `OAuthIdentity`, `FrontendPermission` and `PermissionSet`. `PermissionSet` reads a stored permission column tolerantly (NULL, an empty string, `[]`, numeric strings) and writes a JSON object of integers; `ParsePermissionSet` and `PermissionSet.Text` do the same for a column that is a plain string on its model. `UserGroup.UsersCount` is filled only by a query that selects `users_count` (the admin list does). `Organisation.Members` is the relation behind the members manager, `Organisation.AttachRelations` declares its avatar (`OrganisationAvatarField`), and `Slugify` is the slug rule. `User.FilterScope` answers the admin list scope `FilterByGroup`, and `User.AttachRelations` declares the avatar (`AvatarField`). `User.RegisterFillable` is the registration allow-list; `User.Fillable` is the wider mass-assignment backstop of the admin Users form (cabana intersects it with the form's fields) and of trusted application code.
 - `updates`: the gormigrate migrations, returned by `updates.All()`.
 - `console`: `RequirePasswordChange`, `BackfillHandles` and `APITokenCommands`.
 
@@ -248,4 +248,6 @@ go test ./...           # full run, starts Postgres with testcontainers (needs D
 
 `TestCommandsDocumented` (in `-short`) logs, without failing, each command whose help lacks a description, long help, examples or an option description (`bonfire.CheckDocs`).
 
+`TestRegisterIgnoresProtectedFields` posts organisation, IP and account-flag keys to `/register` and `RegisterUser` and checks that only the allow-listed columns and the server-side IP are stored.
+
 The admin tests (`TestAdmin*`) boot the plugin with the admin API mounted on a database of their own and sign in backend administrators with a chosen permission set; the helpers are in `admin_harness_test.go`.

controllers/registration.go

@@ -22,7 +22,9 @@ import (
 
 // RegisterOptions tunes RegisterUser. Activate creates the account already
 // activated (activate_mode auto, or a caller that activates explicitly).
-// IP, when set, is recorded as created_ip_address and last_ip_address.
+// IP, when set, is recorded as created_ip_address and last_ip_address; it is
+// the only source of those columns, so with an empty IP they stay NULL
+// whatever the fields say.
 type RegisterOptions struct {
 	Activate bool
 	IP       string
@@ -34,12 +36,16 @@ type RegisterOptions struct {
 }
 
 // RegisterUser validates fields against the user model's rules, hashes the
-// password with golem15.user.password.bcrypt_cost (default 10), fills the
-// fillable columns and creates the users row on db, which may be a
-// transaction. Validation failures are returned as Laravel's errors object
-// (attribute to messages) with a nil user and nothing written; they are not
-// written to a response. fields is not modified. The register event is not
-// fired: call FireRegisterEvent once the row is committed.
+// password with golem15.user.password.bcrypt_cost (default 10), fills only
+// the models.User RegisterFillable columns (name, surname, email, password,
+// preferred_locale, marketing_consent) and creates the users row on db,
+// which may be a transaction. Any other key in fields is ignored: an
+// organisation membership and role are set with an explicit update after
+// creation, as an application's onboarding does, and the IP columns come
+// only from opts.IP. Validation failures are returned as Laravel's errors
+// object (attribute to messages) with a nil user and nothing written; they
+// are not written to a response. fields is not modified. The register event
+// is not fired: call FireRegisterEvent once the row is committed.
 func RegisterUser(ctx context.Context, app *backpack.App, db *gorm.DB, fields map[string]any, opts RegisterOptions) (*models.User, map[string][]string, error) {
 	if db == nil {
 		return nil, nil, errors.New("golem15.user: gorm handle is missing")
@@ -64,15 +70,18 @@ func RegisterUser(ctx context.Context, app *backpack.App, db *gorm.DB, fields ma
 		return nil, nil, err
 	}
 	input["password"] = hashed
-	if opts.IP != "" {
-		input["created_ip_address"] = opts.IP
-		input["last_ip_address"] = opts.IP
-	}
 	user := &models.User{}
 	production := app == nil || app.Config == nil || !app.Config.Bool("app.debug")
-	if err := lagoon.Fill(user, user.Fillable(), input, production); err != nil {
+	if err := lagoon.Fill(user, user.RegisterFillable(), input, production); err != nil {
 		return nil, nil, err
 	}
+	// The IP columns are server-set after the fill, so a body value can never
+	// reach them.
+	if opts.IP != "" {
+		created, last := opts.IP, opts.IP
+		user.CreatedIPAddress = &created
+		user.LastIPAddress = &last
+	}
 	user.HasSelfSetPassword = !opts.PasswordLess
 	if opts.Activate {
 		user.IsActivated = true

models/user.go

@@ -81,8 +81,12 @@ func (User) AttachRelations() []attach.Relation {
 	return []attach.Relation{{Name: AvatarField, Public: true}}
 }
 
-// Fillable is the register/update field list. Username, PIN, and GDPR columns
-// stay out of Phase 7 because no ported handler reads them.
+// Fillable is the mass-assignment backstop of the admin Users form and of
+// trusted application code: cabana intersects it with the form's writable
+// fields (and an application's form extensions) and keeps organisation_id
+// protected. Registration does not use it: RegisterUser fills only
+// RegisterFillable. Username, PIN and GDPR columns stay out because no
+// ported handler reads them.
 func (User) Fillable() []string {
 	return []string{
 		"name", "surname", "email", "password", "created_ip_address", "last_ip_address",
@@ -91,6 +95,20 @@ func (User) Fillable() []string {
 	}
 }
 
+// RegisterFillable is the explicit allow-list of the columns a registration
+// body may set (POST /register and controllers.RegisterUser): name, surname,
+// email, password (stored as its hash), preferred_locale and
+// marketing_consent. A registration body can never set organisation_id or
+// organisation_role (membership is the administrator's or the
+// application's), created_ip_address or last_ip_address (server-set from
+// RegisterOptions.IP only), is_onboarded (application-owned onboarding
+// state) or must_change_password (set by an administrator or the
+// require-password-change command). The activation, reset, address proof,
+// handle and permission columns are never fillable at all.
+func (User) RegisterFillable() []string {
+	return []string{"name", "surname", "email", "password", "preferred_locale", "marketing_consent"}
+}
+
 // Hidden mirrors PHP User::$hidden for the secrets this model stores.
 func (User) Hidden() []string {
 	return []string{"password", "reset_password_code", "activation_code"}

register_test.go

@@ -248,6 +248,81 @@ func TestRegisterEventPayload(t *testing.T) {
 	}
 }
 
+// TestRegisterIgnoresProtectedFields: a registration body sets only the
+// RegisterFillable columns. Organisation membership, the IP columns and the
+// account flags in the body are ignored, over HTTP and through RegisterUser,
+// and the IP columns come only from the server-side address.
+func TestRegisterIgnoresProtectedFields(t *testing.T) {
+	app, gdb := sessionApp(t, "0s")
+	disableRegisterThrottle(t, app)
+	org := &models.Organisation{Name: "Mass Assign Org", Slug: "mass-assign-org"}
+	if err := gdb.Create(org).Error; err != nil {
+		t.Fatal(err)
+	}
+	protected := func(email string) map[string]any {
+		return map[string]any{
+			"email": email, "password": "correct-horse", "password_confirmation": "correct-horse", "name": "Mal", "surname": "Lory",
+			"preferred_locale": "pl", "marketing_consent": true,
+			"organisation_id": org.ID, "organisation_role": "owner",
+			"created_ip_address": "203.0.113.9", "last_ip_address": "203.0.113.9",
+			"is_onboarded": true, "must_change_password": true,
+		}
+	}
+	check := func(t *testing.T, email string, wantIP *string) {
+		t.Helper()
+		var stored models.User
+		if err := gdb.Where("email = ?", email).Take(&stored).Error; err != nil {
+			t.Fatal(err)
+		}
+		if stored.Name == nil || *stored.Name != "Mal" || stored.Surname == nil || *stored.Surname != "Lory" ||
+			stored.PreferredLocale == nil || *stored.PreferredLocale != "pl" || !stored.MarketingConsent {
+			t.Fatalf("allowed columns not stored: %+v", stored)
+		}
+		if stored.OrganisationID != nil || stored.OrganisationRole != nil {
+			t.Fatalf("organisation from the body: id %v role %v", stored.OrganisationID, stored.OrganisationRole)
+		}
+		if stored.IsOnboarded || stored.MustChangePassword {
+			t.Fatalf("account flags from the body: onboarded %v must_change %v", stored.IsOnboarded, stored.MustChangePassword)
+		}
+		for name, got := range map[string]*string{"created_ip_address": stored.CreatedIPAddress, "last_ip_address": stored.LastIPAddress} {
+			switch {
+			case wantIP == nil && got != nil:
+				t.Fatalf("%s = %q, want NULL", name, *got)
+			case wantIP != nil && (got == nil || *got != *wantIP):
+				t.Fatalf("%s = %v, want %q", name, got, *wantIP)
+			}
+		}
+	}
+
+	t.Run("http", func(t *testing.T) {
+		body, err := json.Marshal(protected("mass-http@example.test"))
+		if err != nil {
+			t.Fatal(err)
+		}
+		rec := postRegister(t, app, "192.0.2.44", string(body))
+		if rec.Code != http.StatusOK {
+			t.Fatalf("register %d %s", rec.Code, rec.Body.String())
+		}
+		ip := "192.0.2.44"
+		check(t, "mass-http@example.test", &ip)
+	})
+	t.Run("RegisterUser with IP", func(t *testing.T) {
+		_, errs, err := controllers.RegisterUser(t.Context(), app, gdb, protected("mass-ip@example.test"), controllers.RegisterOptions{IP: "198.51.100.7"})
+		if err != nil || len(errs) != 0 {
+			t.Fatalf("RegisterUser = %v, %v", errs, err)
+		}
+		ip := "198.51.100.7"
+		check(t, "mass-ip@example.test", &ip)
+	})
+	t.Run("RegisterUser without IP", func(t *testing.T) {
+		_, errs, err := controllers.RegisterUser(t.Context(), app, gdb, protected("mass-noip@example.test"), controllers.RegisterOptions{})
+		if err != nil || len(errs) != 0 {
+			t.Fatalf("RegisterUser = %v, %v", errs, err)
+		}
+		check(t, "mass-noip@example.test", nil)
+	})
+}
+
 func TestRegisterImportDirection(t *testing.T) {
 	cmd := exec.Command("go", "list", "-deps", "./...")
 	cmd.Dir = moduleRoot(t)