diff --git a/plugin/manager.go b/plugin/manager.go index 5048f2328..438cd1eb9 100644 --- a/plugin/manager.go +++ b/plugin/manager.go @@ -323,6 +323,20 @@ func (m *Manager) initializeSingleUserPlugin(userCtx compat.UserContext, p compa m.instances[pluginConf.ID] = instance if compat.HasSupport(instance, compat.Messenger) { + if pluginConf.ApplicationID == 0 { + // The Messenger capability was added after this plugin was first + // initialized for the user, so no internal application exists yet. + // Create one now, otherwise messages would be stored with + // application_id = 0 and become orphaned (not shown, not deletable). + app, err := m.createInternalApplication(info, userID) + if err != nil { + return err + } + pluginConf.ApplicationID = app.ID + if err := m.db.UpdatePluginConf(pluginConf); err != nil { + return err + } + } instance.SetMessageHandler(redirectToChannel{ ApplicationID: pluginConf.ApplicationID, UserID: pluginConf.UserID, @@ -399,15 +413,8 @@ func (m *Manager) createPluginConf(instance compat.PluginInstance, info compat.I pluginConf.Config, _ = yaml.Marshal(instance.DefaultConfig()) } if compat.HasSupport(instance, compat.Messenger) { - tokenPublic, _ := auth.GenerateApplicationToken() - app := &model.Application{ - Token: tokenPublic, - Name: info.String(), - UserID: userID, - Internal: true, - Description: fmt.Sprintf("auto generated application for %s", info.ModulePath), - } - if err := m.db.CreateApplication(app); err != nil { + app, err := m.createInternalApplication(info, userID) + if err != nil { return nil, err } pluginConf.ApplicationID = app.ID @@ -417,3 +424,20 @@ func (m *Manager) createPluginConf(instance compat.PluginInstance, info compat.I } return pluginConf, nil } + +// createInternalApplication creates the auto generated internal application a +// Messenger plugin uses to publish its messages. +func (m *Manager) createInternalApplication(info compat.Info, userID uint) (*model.Application, error) { + tokenPublic, _ := auth.GenerateApplicationToken() + app := &model.Application{ + Token: tokenPublic, + Name: info.String(), + UserID: userID, + Internal: true, + Description: fmt.Sprintf("auto generated application for %s", info.ModulePath), + } + if err := m.db.CreateApplication(app); err != nil { + return nil, err + } + return app, nil +} diff --git a/plugin/manager_test.go b/plugin/manager_test.go index 63811cffa..62eab53f2 100644 --- a/plugin/manager_test.go +++ b/plugin/manager_test.go @@ -443,6 +443,105 @@ func TestNewManager_InternalApplicationManagement(t *testing.T) { } } +func TestNewManager_MessengerAddedAfterInit_createsApplication(t *testing.T) { + db := testdb.NewDBWithDefaultUser(t) + + // Simulate a plugin conf that was created on a previous startup when the + // plugin did not yet implement the Messenger interface: it has no + // associated internal application (ApplicationID == 0). + assert.NoError(t, db.CreatePluginConf(&model.PluginConf{ + UserID: 1, + ModulePath: mock.ModulePath, + Enabled: true, + Token: auth.GeneratePluginToken(), + })) + + manager, err := NewManager(db, "", nil, nil) + assert.Nil(t, err) + assert.Nil(t, manager.LoadPlugin(new(mock.Plugin))) + // The mock plugin supports Messenger, so re-initializing must back-fill the + // missing internal application instead of leaving ApplicationID at 0. + assert.Nil(t, manager.InitializeForUserID(1)) + + conf, err := db.GetPluginConfByUserAndPath(1, mock.ModulePath) + assert.NoError(t, err) + if assert.NotNil(t, conf) { + assert.NotZero(t, conf.ApplicationID, "an internal application should have been created for the messenger plugin") + + app, err := db.GetApplicationByID(conf.ApplicationID) + assert.NoError(t, err) + if assert.NotNil(t, app) { + assert.True(t, app.Internal) + assert.Equal(t, uint(1), app.UserID) + } + } +} + +// failingDB wraps a real test database so that individual operations can be +// forced to fail, allowing the error branches around internal-application +// creation to be exercised. +type failingDB struct { + *testdb.Database + failCreateApplication error + failUpdatePluginConf error +} + +func (d *failingDB) CreateApplication(app *model.Application) error { + if d.failCreateApplication != nil { + return d.failCreateApplication + } + return d.Database.CreateApplication(app) +} + +func (d *failingDB) UpdatePluginConf(conf *model.PluginConf) error { + if d.failUpdatePluginConf != nil { + return d.failUpdatePluginConf + } + return d.Database.UpdatePluginConf(conf) +} + +func seedMessengerConfWithoutApplication(t *testing.T, db Database) { + t.Helper() + assert.NoError(t, db.CreatePluginConf(&model.PluginConf{ + UserID: 1, + ModulePath: mock.ModulePath, + Enabled: true, + Token: auth.GeneratePluginToken(), + })) +} + +func TestNewManager_MessengerAddedAfterInit_createApplicationError(t *testing.T) { + db := &failingDB{ + Database: testdb.NewDBWithDefaultUser(t), + failCreateApplication: errors.New("create application failed"), + } + seedMessengerConfWithoutApplication(t, db) + + manager, err := NewManager(db, "", nil, nil) + assert.Nil(t, err) + assert.Nil(t, manager.LoadPlugin(new(mock.Plugin))) + + // Back-filling the missing internal application must surface the database + // error instead of silently continuing with ApplicationID == 0. + assert.EqualError(t, manager.InitializeForUserID(1), "create application failed") +} + +func TestNewManager_MessengerAddedAfterInit_updatePluginConfError(t *testing.T) { + db := &failingDB{ + Database: testdb.NewDBWithDefaultUser(t), + failUpdatePluginConf: errors.New("update plugin conf failed"), + } + seedMessengerConfWithoutApplication(t, db) + + manager, err := NewManager(db, "", nil, nil) + assert.Nil(t, err) + assert.Nil(t, manager.LoadPlugin(new(mock.Plugin))) + + // Persisting the back-filled ApplicationID may fail; that error must be + // propagated as well. + assert.EqualError(t, manager.InitializeForUserID(1), "update plugin conf failed") +} + func TestPluginFileLoadError(t *testing.T) { err := pluginFileLoadError{Filename: "test.so", UnderlyingError: errors.New("test error")} assert.Error(t, err) diff --git a/plugin/messagehandler.go b/plugin/messagehandler.go index 01f01c5d6..16907c66d 100644 --- a/plugin/messagehandler.go +++ b/plugin/messagehandler.go @@ -1,6 +1,7 @@ package plugin import ( + "errors" "time" "github.com/gotify/server/v2/model" @@ -21,6 +22,13 @@ type MessageWithUserID struct { // SendMessage sends a message to the underlying message channel. func (c redirectToChannel) SendMessage(msg compat.Message) error { + if c.ApplicationID == 0 { + // Final safety net: the internal application should always be set up by + // Manager.initializeSingleUserPlugin. If it somehow isn't, refuse the + // message instead of storing it with application_id = 0, where it would + // be orphaned (not shown in the UI and not deletable). + return errors.New("plugin messenger has no associated internal application") + } c.Messages <- MessageWithUserID{ Message: model.MessageExternal{ ApplicationID: c.ApplicationID, diff --git a/plugin/messagehandler_test.go b/plugin/messagehandler_test.go new file mode 100644 index 000000000..c3b8639cf --- /dev/null +++ b/plugin/messagehandler_test.go @@ -0,0 +1,31 @@ +package plugin + +import ( + "testing" + + "github.com/gotify/server/v2/plugin/compat" + "github.com/stretchr/testify/assert" +) + +func TestRedirectToChannel_SendMessage_rejectsMissingApplication(t *testing.T) { + messages := make(chan MessageWithUserID, 1) + handler := redirectToChannel{ApplicationID: 0, UserID: 1, Messages: messages} + + err := handler.SendMessage(compat.Message{Message: "orphan"}) + + assert.Error(t, err) + assert.Empty(t, messages, "no message should be queued when the internal application is missing") +} + +func TestRedirectToChannel_SendMessage_forwardsWithApplication(t *testing.T) { + messages := make(chan MessageWithUserID, 1) + handler := redirectToChannel{ApplicationID: 7, UserID: 3, Messages: messages} + + assert.NoError(t, handler.SendMessage(compat.Message{Message: "hi", Title: "t"})) + + got := <-messages + assert.Equal(t, uint(7), got.Message.ApplicationID) + assert.Equal(t, uint(3), got.UserID) + assert.Equal(t, "hi", got.Message.Message) + assert.Equal(t, "t", got.Message.Title) +}