From e0b5ffe20d27a0aa759ae3ae7dcd3729df8dbb50 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Thu, 22 Oct 2026 03:07:22 +0200 Subject: [PATCH] fix: log /register listener failures instead of discarding them A failing user.Register or user.Created listener is logged at Error level with the user ID; the committed registration still succeeds. --- README.md | 8 ++++---- controllers/api_controller.go | 10 ++++++++-- controllers/registration.go | 9 +++++---- register_test.go | 22 ++++++++++++++++++++++ 4 files changed, 39 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index f8c9e47..6272e8b 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,7 @@ It owns the `users` table extensions, `user_throttle`, `jwt_blacklist`, `user_ap - **User lighthouse authorizer.** Boot calls `classes.RegisterUserAuthorizer`: `lighthouse.From` then `Registry.Register("user", …)` if the namespace is free. `classes.AuthorizeUser` allows only `user:{id}` for that user (`lighthouse.ChannelID` on the two-segment name). - **Shared API tokens.** `user_api_tokens` stores one hashed personal/OAuth credential per user for the whole application. The `user.api_token` guard authenticates it for any plugin; `user.scope:` limits delegated access and `user.permission:` checks the user's current group permissions. `POST/GET/DELETE /tokens` manage personal tokens, and `user:token:create`, `user:token:list` and `user:token:revoke` provide the CLI equivalent. A raw secret is shown only when created. - **Mail templates.** Activation, reset, reactivation and invitation mails in Polish and English (`golem15.user::mail.*`, layout `golem15.user::mail.layouts.user`); the locale follows the user (`ResolveMailLocale`). The invitation (`golem15.user::mail.invite`) is sent when an administrator creates a user with "Send invitation by email" set; it carries the user's login and an activation link, never the password. -- **Extension events.** `RegisterEvent` fires after a user row is created; its `Payload` is a copy of the register input (extra keys a client sent included, `password` and `password_confirmation` never). `CreatedEvent` (`classes.CreatedEventName` `user.Created`) fires from API register and admin create. `DeletingEvent` (`classes.DeletingEventName` `user.Deleting`) fires from `FormAfterDelete` before `ForceDeleteCleanup` / unscoped delete, inside the same transaction. `GetApiArrayEvent` lets other plugins merge keys into the user payload. +- **Extension events.** `RegisterEvent` fires after a user row is created; its `Payload` is a copy of the register input (extra keys a client sent included, `password` and `password_confirmation` never), so it is untrusted client input. `CreatedEvent` (`classes.CreatedEventName` `user.Created`) fires from API register and admin create. `/register` logs a failing `RegisterEvent` or `CreatedEvent` listener at Error level and continues: a committed registration is never undone or failed by a listener. `DeletingEvent` (`classes.DeletingEventName` `user.Deleting`) fires from `FormAfterDelete` before `ForceDeleteCleanup` / unscoped delete, inside the same transaction. `GetApiArrayEvent` lets other plugins merge keys into the user payload. - **Registration for other plugins.** `RegisterUser`, `FireRegisterEvent`, `IssueToken` and `APIArray` are the steps `POST /register` runs, exported so another plugin (a first-run setup endpoint, for example) creates a user with the same validation, hashing, event, token and payload. - **OAuth providers.** `GET /oauth-providers` lists the social sign-in providers a social-login plugin has enabled, read on every request from its published `classes.OAuthProviderSource`. Without one it answers the empty list, as before. - **Sign-in helpers for social login.** `RegisterOptions.PasswordLess`, `FinishLogin`, `RegistrationSettings`, `RegistrationsFromIP`, `RefreshTTL` and `RemoteIP` let a social-login plugin create accounts and finish logins with the user plugin's own rules instead of copies. @@ -169,8 +169,8 @@ 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 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. + - `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` logs it and continues). `Payload` carries every extra key the client sent, so treat it as untrusted. Fire it after the user row is committed. + - `FireCreatedEvent(ctx, app, user)` fires `CreatedEvent` (`user.Created`) and returns the listeners' error; `/register` logs it and continues. - `FireDeletingEvent(ctx, app, user, tx)` fires `DeletingEvent` (`user.Deleting`) inside the delete transaction. - `FireDeactivatingEvent(ctx, app, user, tx)` fires `DeactivatingEvent` (`user.Deactivating`) inside the deactivation transaction. The admin deactivate bulk action fires it after `DeactivateUsers` for every selected user that was not deactivated yet; a listener error rolls the action back. - `IssueToken(app, user, r)` mints the session JWT as `/register` and `/login` do. @@ -248,6 +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. +`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; `TestRegisterListenerErrors` checks that failing register and created listeners do not fail the registration. 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`. diff --git a/controllers/api_controller.go b/controllers/api_controller.go index c57451f..781db98 100644 --- a/controllers/api_controller.go +++ b/controllers/api_controller.go @@ -682,8 +682,14 @@ func Register(app *backpack.App) http.HandlerFunc { }) return } - _ = FireRegisterEvent(r.Context(), app, user, fields) - _ = FireCreatedEvent(r.Context(), app, user) + // The row is committed: a failing listener is logged and never undoes + // or fails the registration, as in Winter. + if err := FireRegisterEvent(r.Context(), app, user, fields); err != nil { + pluginLog.Error("golem15.user: register listener failed", "user_id", user.ID, "err", err) + } + if err := FireCreatedEvent(r.Context(), app, user); err != nil { + pluginLog.Error("golem15.user: created listener failed", "user_id", user.ID, "err", err) + } ctx := r.Context() if mode == "user" { vars := mailVars(displayName(user), "", "") diff --git a/controllers/registration.go b/controllers/registration.go index bc35863..30aac92 100644 --- a/controllers/registration.go +++ b/controllers/registration.go @@ -133,8 +133,9 @@ func FinishLogin(ctx context.Context, app *backpack.App, db *gorm.DB, user *mode // FireRegisterEvent fires classes.RegisterEvent for user with Payload set // to a copy of fields without password and password_confirmation, and -// returns the listeners' error. /register ignores that error, as Winter -// does not stop a registration for a failing listener. +// returns the listeners' error. /register logs that error and continues, as +// Winter does not stop a registration for a failing listener. Payload is +// untrusted client input: it carries every extra key the client sent. func FireRegisterEvent(ctx context.Context, app *backpack.App, user *models.User, fields map[string]any) error { if app == nil || app.Events == nil { return nil @@ -149,8 +150,8 @@ func FireRegisterEvent(ctx context.Context, app *backpack.App, user *models.User } // FireCreatedEvent fires classes.CreatedEvent (user.Created) for user and -// returns the listeners' error. /register ignores that error, as it does -// for FireRegisterEvent. +// returns the listeners' error. /register logs that error and continues, as +// it does for FireRegisterEvent. func FireCreatedEvent(ctx context.Context, app *backpack.App, user *models.User) error { if app == nil || app.Events == nil { return nil diff --git a/register_test.go b/register_test.go index 90c1039..44c1c4d 100644 --- a/register_test.go +++ b/register_test.go @@ -2,6 +2,7 @@ package user import ( "encoding/json" + "errors" "net/http" "net/http/httptest" "os" @@ -248,6 +249,27 @@ func TestRegisterEventPayload(t *testing.T) { } } +// TestRegisterListenerErrors: failing register and created listeners are +// logged and neither fail nor undo the registration. +func TestRegisterListenerErrors(t *testing.T) { + app, gdb := sessionApp(t, "0s") + disableRegisterThrottle(t, app) + app.Events.Listen[*classes.RegisterEvent]("failing-register", func(context.Context, *classes.RegisterEvent) error { + return errors.New("register listener down") + }) + app.Events.Listen[*classes.CreatedEvent]("failing-created", func(context.Context, *classes.CreatedEvent) error { + return errors.New("created listener down") + }) + rec := postJSON(t, controllers.Register(app), "/_user/api/v1/register", `{"email":"listener-fail@example.test","password":"correct-horse"}`) + if rec.Code != http.StatusOK || !strings.Contains(rec.Body.String(), `"token"`) { + t.Fatalf("register %d %s", rec.Code, rec.Body.String()) + } + var count int64 + if err := gdb.Model(&models.User{}).Where("email = ?", "listener-fail@example.test").Count(&count).Error; err != nil || count != 1 { + t.Fatalf("stored users = %d, %v", count, err) + } +} + // 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,