From c1c29b1e373e5e876fbc90a519785ed435805bb1 Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Mon, 20 Jul 2026 10:44:47 +0200 Subject: [PATCH 1/2] Add multi-Matrix-server backend groundwork (registry, namespaced KV, v3 migration) Restructure the plugin internals so a Matrix homeserver is modeled as an entry in a managed registry keyed by a stable serverID, laying the groundwork for supporting multiple homeservers. No cross-server routing yet: the bridge still resolves a single server. Zero behavior change for a single-server operator. - Registry: servers_config KV entry (ServerConfig) reconciled from the flat plugin.json config; serverID derived deterministically from the homeserver hostname so records are re-adopted if a server is re-created with the same URL. - KV namespacing: per-server keys become _; channel_mapping_ value becomes a []ChannelServerMapping (server association in the value). - Client registry: p.matrixClient -> map[serverID]*matrix.Client with accessors. - v3 KV migration: namespaces existing per-server keys and converts channel_mapping values; deterministic, idempotent, and aborts (without bumping the version) on partial failure so it safely retries. Legacy v1/v2 steps stay un-namespaced. - Per-server username prefix resolved from the registry (source of truth). - Tests: registry reconcile/derivation, migration (fresh/upgrade/idempotent/ reset/failure-abort), per-server isolation, corrupt-value handling, command handlers, and a two-container live-Synapse integration suite. Docs under spec/. --- server/bridge_utils.go | 85 ++- server/command/command.go | 68 ++- server/command/command_test.go | 121 +++- server/configuration.go | 12 +- server/dm_room_creation_test.go | 25 +- server/dm_support_test.go | 9 +- server/hooks.go | 24 +- server/matrix_mentions_integration_test.go | 7 +- server/matrix_util.go | 9 +- server/matrix_webhook.go | 12 +- server/migrations.go | 236 +++++++- server/migrations_test.go | 518 ++++++++++++++++-- server/multi_server_integration_test.go | 234 ++++++++ server/plugin.go | 95 +++- server/plugin_integration_test.go | 30 +- server/servers.go | 165 ++++++ server/servers_test.go | 425 ++++++++++++++ server/store/kvstore/constants.go | 44 +- server/store/kvstore/schema.go | 120 ++++ server/store/kvstore/schema_test.go | 48 ++ server/sync_to_matrix.go | 8 +- server/sync_to_matrix_integration_test.go | 6 +- server/sync_to_matrix_test.go | 2 +- server/sync_to_mattermost.go | 19 +- server/sync_to_mattermost_test.go | 24 +- server/testhelpers_test.go | 79 ++- server/thread_mapping_test.go | 6 +- server/user_remote_detection_test.go | 22 +- ...-16-multi-matrix-server-support-phase-1.md | 133 +++++ ...-16-multi-matrix-server-support-phase-2.md | 67 +++ ...-16-multi-matrix-server-support-phase-3.md | 68 +++ ...-16-multi-matrix-server-support-phase-4.md | 66 +++ ...-16-multi-matrix-server-support-phase-5.md | 55 ++ ...-16-multi-matrix-server-support-phase-6.md | 46 ++ webapp/package-lock.json | 1 - 35 files changed, 2641 insertions(+), 248 deletions(-) create mode 100644 server/multi_server_integration_test.go create mode 100644 server/servers.go create mode 100644 server/servers_test.go create mode 100644 server/store/kvstore/schema.go create mode 100644 server/store/kvstore/schema_test.go create mode 100644 spec/2026-07-16-multi-matrix-server-support-phase-1.md create mode 100644 spec/2026-07-16-multi-matrix-server-support-phase-2.md create mode 100644 spec/2026-07-16-multi-matrix-server-support-phase-3.md create mode 100644 spec/2026-07-16-multi-matrix-server-support-phase-4.md create mode 100644 spec/2026-07-16-multi-matrix-server-support-phase-5.md create mode 100644 spec/2026-07-16-multi-matrix-server-support-phase-6.md diff --git a/server/bridge_utils.go b/server/bridge_utils.go index e986138..413fc91 100644 --- a/server/bridge_utils.go +++ b/server/bridge_utils.go @@ -37,6 +37,7 @@ type BridgeUtilsConfig struct { API plugin.API KVStore kvstore.KVStore MatrixClient *matrix.Client + ServerID string RemoteID string MaxProfileImageSize int64 MaxFileSize int64 @@ -49,6 +50,7 @@ type BridgeUtils struct { API plugin.API kvstore kvstore.KVStore matrixClient *matrix.Client + serverID string remoteID string maxProfileImageSize int64 maxFileSize int64 @@ -62,6 +64,7 @@ func NewBridgeUtils(config BridgeUtilsConfig) *BridgeUtils { API: config.API, kvstore: config.KVStore, matrixClient: config.MatrixClient, + serverID: config.ServerID, remoteID: config.RemoteID, maxProfileImageSize: config.MaxProfileImageSize, maxFileSize: config.MaxFileSize, @@ -71,17 +74,33 @@ func NewBridgeUtils(config BridgeUtilsConfig) *BridgeUtils { // Shared utility methods that both bridge types need -// GetMatrixRoomID retrieves the Matrix room ID for a given Mattermost channel ID +// GetMatrixRoomID retrieves the Matrix room ID for a given Mattermost channel ID. +// An unmapped channel yields ("", nil): the plugin KV API returns no error for a +// missing key, so the value is simply empty. A real KV read failure or a corrupt +// (unparseable) value is returned as an error rather than being masked as +// unmapped, which would silently mis-route or drop messages. func (s *BridgeUtils) GetMatrixRoomID(channelID string) (string, error) { - roomID, err := s.kvstore.Get(kvstore.BuildChannelMappingKey(channelID)) + data, err := s.kvstore.Get(kvstore.BuildChannelMappingKey(channelID)) if err != nil { - // KV store error (typically key not found) - unmapped channels are expected - return "", nil + return "", errors.Wrap(err, "failed to read channel mapping") } - return string(roomID), nil + mappings, err := kvstore.ParseChannelServerMappings(data) + if err != nil { + // After the v3 migration every stored value is well-formed JSON, so a + // parse failure here is a corrupt record, not an unmapped channel. + return "", errors.Wrapf(err, "corrupt channel mapping value for channel %s", channelID) + } + return kvstore.RoomIDForServer(mappings, s.serverID), nil } func (s *BridgeUtils) setChannelRoomMapping(channelID, matrixRoomIdentifier string) error { + // Guard against persisting a mapping with an empty serverID, which would be + // unroutable and would corrupt the room_mapping_ reverse key. This should not + // happen once the registry is reconciled, but fail loudly rather than write it. + if s.serverID == "" { + return errors.New("cannot store channel mapping: server ID not initialized") + } + // Always resolve to room ID for consistent forward mapping storage var roomID string var err error @@ -94,14 +113,29 @@ func (s *BridgeUtils) setChannelRoomMapping(channelID, matrixRoomIdentifier stri roomID = matrixRoomIdentifier } - // Store forward mapping: channel_mapping_ -> room_id (always room ID) - err = s.kvstore.Set(kvstore.BuildChannelMappingKey(channelID), []byte(roomID)) + // Store forward mapping: channel_mapping_ -> [{serverID, room_id}]. + // Upsert only this server's entry so a channel bridged to multiple homeservers + // keeps the others' mappings intact. + existingData, err := s.kvstore.Get(kvstore.BuildChannelMappingKey(channelID)) + if err != nil { + return errors.Wrap(err, "failed to read existing channel room mapping") + } + mappings, err := kvstore.ParseChannelServerMappings(existingData) + if err != nil { + return errors.Wrap(err, "failed to parse existing channel room mapping") + } + mappings = kvstore.UpsertChannelServerMapping(mappings, s.serverID, roomID) + mappingValue, err := kvstore.MarshalChannelServerMappings(mappings) + if err != nil { + return errors.Wrap(err, "failed to marshal channel room mapping") + } + err = s.kvstore.Set(kvstore.BuildChannelMappingKey(channelID), mappingValue) if err != nil { return errors.Wrap(err, "failed to store channel room mapping") } // Store reverse mapping for the room ID - err = s.kvstore.Set(kvstore.BuildRoomMappingKey(roomID), []byte(channelID)) + err = s.kvstore.Set(kvstore.BuildRoomMappingKey(s.serverID, roomID), []byte(channelID)) if err != nil { return errors.Wrap(err, "failed to store reverse room mapping") } @@ -109,7 +143,7 @@ func (s *BridgeUtils) setChannelRoomMapping(channelID, matrixRoomIdentifier stri // If we started with an alias, also create reverse mapping for the alias // This allows lookups by both alias and room ID if strings.HasPrefix(matrixRoomIdentifier, "#") && roomID != matrixRoomIdentifier { - err = s.kvstore.Set(kvstore.BuildRoomMappingKey(matrixRoomIdentifier), []byte(channelID)) + err = s.kvstore.Set(kvstore.BuildRoomMappingKey(s.serverID, matrixRoomIdentifier), []byte(channelID)) if err != nil { s.logger.LogWarn("Failed to create alias reverse mapping", "channel_id", channelID, "room_alias", matrixRoomIdentifier, "error", err) } else { @@ -124,6 +158,35 @@ func (s *BridgeUtils) getConfiguration() *configuration { return s.configGetter.getConfiguration() } +// matrixUsernamePrefix returns the username prefix for this bridge's Matrix +// server, resolved from the managed server registry, which is the source of +// truth for per-server settings. reconcileServerConfig always populates each +// entry's prefix (defaulting to DefaultMatrixUsernamePrefix) before bridges run, +// so a configured server always resolves here. The static default is only for +// the degenerate case of no registered server (e.g. before the first reconcile). +// The registry is read live so prefix changes take effect without recreating the +// bridge. +func (s *BridgeUtils) matrixUsernamePrefix() string { + data, err := s.kvstore.Get(kvstore.KeyServersConfig) + if err != nil { + // A transient registry read failure must not silently change the prefix: + // a wrong prefix splits Matrix-user identity (ghosts created and matched + // under a different prefix). Surface it loudly, mirroring GetMatrixRoomID. + s.logger.LogError("Failed to read server registry for username prefix; using default", "server_id", s.serverID, "error", err) + return DefaultMatrixUsernamePrefix + } + servers, err := kvstore.ParseServersConfig(data) + if err != nil { + s.logger.LogError("Corrupt server registry; using default username prefix", "server_id", s.serverID, "error", err) + return DefaultMatrixUsernamePrefix + } + if server, ok := kvstore.ServerConfigForID(servers, s.serverID); ok && server.UsernamePrefix != "" { + return server.UsernamePrefix + } + // No registry yet (before the first reconcile) or no entry for this server. + return DefaultMatrixUsernamePrefix +} + func (s *BridgeUtils) extractMattermostMetadata(event MatrixEvent) (postID string, remoteID string) { if event.Content != nil { if id, ok := event.Content["mattermost_post_id"].(string); ok { @@ -284,7 +347,7 @@ func (s *BridgeUtils) getMattermostUsernameFromMatrix(matrixUserID string) strin mattermostUserID = ghostMattermostUserID } else { // Check if we have a mapping for this regular Matrix user - userMapKey := "matrix_user_" + matrixUserID + userMapKey := kvstore.BuildMatrixUserKey(s.serverID, matrixUserID) userIDBytes, err := s.kvstore.Get(userMapKey) if err != nil || len(userIDBytes) == 0 { s.logger.LogDebug("No Mattermost user found for Matrix mention", "matrix_user_id", matrixUserID) @@ -435,7 +498,7 @@ func (s *BridgeUtils) reconstructMatrixUserIDFromUsername(mattermostUsername str // We need to reverse this to get "@username:server.com" config := s.configGetter.getConfiguration() - prefix := config.GetMatrixUsernamePrefixForServer(config.GetMatrixServerURL()) + prefix := s.matrixUsernamePrefix() // Check if username has the expected prefix expectedPrefix := prefix + ":" diff --git a/server/command/command.go b/server/command/command.go index 57e099a..7949c33 100644 --- a/server/command/command.go +++ b/server/command/command.go @@ -18,7 +18,6 @@ import ( type Configuration interface { GetMatrixServerURL() string GetMatrixServerName() string - GetMatrixUsernamePrefixForServer(serverURL string) string } // MigrationResult holds the results of a migration operation @@ -54,6 +53,9 @@ type PluginAccessor interface { // Shared channel access GetRemoteID() string + // Server registry access + GetServerID() string + // Migration access RunKVStoreMigrations() error RunKVStoreMigrationsWithResults() (*MigrationResult, error) @@ -388,8 +390,12 @@ func (c *Handler) executeMapCommand(args *model.CommandArgs, roomIdentifier stri } // Save both directions of the mapping + serverID := c.plugin.GetServerID() mappingKey := kvstore.BuildChannelMappingKey(args.ChannelId) - err := c.kvstore.Set(mappingKey, []byte(roomIdentifier)) + mappingValue, err := kvstore.BuildSingleChannelMapping(serverID, roomIdentifier) + if err == nil { + err = c.kvstore.Set(mappingKey, mappingValue) + } if err != nil { c.client.Log.Error("Failed to save channel mapping", "error", err, "channel_id", args.ChannelId, "room_identifier", roomIdentifier) return &model.CommandResponse{ @@ -398,8 +404,8 @@ func (c *Handler) executeMapCommand(args *model.CommandArgs, roomIdentifier stri } } - // Store reverse mapping: room_mapping_ -> channelID - roomMappingKey := kvstore.BuildRoomMappingKey(roomIdentifier) + // Store reverse mapping: room_mapping__ -> channelID + roomMappingKey := kvstore.BuildRoomMappingKey(serverID, roomIdentifier) err = c.kvstore.Set(roomMappingKey, []byte(args.ChannelId)) if err != nil { c.client.Log.Error("Failed to save room mapping", "error", err, "room_identifier", roomIdentifier, "channel_id", args.ChannelId) @@ -409,7 +415,7 @@ func (c *Handler) executeMapCommand(args *model.CommandArgs, roomIdentifier stri // If roomIdentifier is an alias, also resolve to room ID and store that mapping if strings.HasPrefix(roomIdentifier, "#") { if resolvedRoomID, err := matrixClient.ResolveRoomAlias(roomIdentifier); err == nil { - roomIDMappingKey := kvstore.BuildRoomMappingKey(resolvedRoomID) + roomIDMappingKey := kvstore.BuildRoomMappingKey(serverID, resolvedRoomID) if err := c.kvstore.Set(roomIDMappingKey, []byte(args.ChannelId)); err != nil { c.client.Log.Error("Failed to save room ID mapping", "error", err, "room_id", resolvedRoomID, "channel_id", args.ChannelId) } @@ -503,9 +509,29 @@ func (c *Handler) executeUnmapCommand(args *model.CommandArgs) *model.CommandRes } // Check if this channel has a Matrix room mapping + serverID := c.plugin.GetServerID() channelMappingKey := kvstore.BuildChannelMappingKey(args.ChannelId) roomIDBytes, err := c.kvstore.Get(channelMappingKey) - if err != nil { + + // A corrupt (unparseable) value is distinct from an unmapped channel. Clear + // the bad record so the admin can recover with /matrix map, rather than being + // told the channel is not mapped with no way to fix it. + if err == nil && len(roomIDBytes) > 0 { + if _, parseErr := kvstore.ParseChannelServerMappings(roomIDBytes); parseErr != nil { + c.client.Log.Error("Corrupt channel mapping value; clearing it", "error", parseErr, "channel_id", args.ChannelId) + if delErr := c.kvstore.Delete(channelMappingKey); delErr != nil { + c.client.Log.Error("Failed to clear corrupt channel mapping", "error", delErr, "channel_id", args.ChannelId) + } + return &model.CommandResponse{ + ResponseType: model.CommandResponseTypeEphemeral, + Text: fmt.Sprintf("⚠️ **Corrupt Mapping Cleared**\n\nChannel `%s` had an unreadable Matrix room mapping, which has been removed. Use `/matrix map` to remap it if needed.", channelName), + } + } + } + + mappings, _ := kvstore.ParseChannelServerMappings(roomIDBytes) + matrixRoomIdentifier := kvstore.RoomIDForServer(mappings, serverID) + if err != nil || matrixRoomIdentifier == "" { // Key not found is expected for unmapped channels return &model.CommandResponse{ ResponseType: model.CommandResponseTypeEphemeral, @@ -513,8 +539,6 @@ func (c *Handler) executeUnmapCommand(args *model.CommandArgs) *model.CommandRes } } - matrixRoomIdentifier := string(roomIDBytes) - // Clear the Matrix room state to prevent fallback lookups - this is critical matrixClient := c.plugin.GetMatrixClient() if matrixClient == nil { @@ -545,7 +569,7 @@ func (c *Handler) executeUnmapCommand(args *model.CommandArgs) *model.CommandRes } // Remove the room->channel mapping - roomMappingKey := kvstore.BuildRoomMappingKey(matrixRoomIdentifier) + roomMappingKey := kvstore.BuildRoomMappingKey(serverID, matrixRoomIdentifier) if err := c.kvstore.Delete(roomMappingKey); err != nil { c.client.Log.Warn("Failed to remove room mapping", "error", err, "room_identifier", matrixRoomIdentifier, "channel_id", args.ChannelId) // Continue - the main mapping was removed @@ -634,17 +658,22 @@ func (c *Handler) executeCreateRoomCommand(args *model.CommandArgs, roomName str } // Automatically map the created room to this channel (both directions) + serverID := c.plugin.GetServerID() mappingKey := kvstore.BuildChannelMappingKey(args.ChannelId) - if err := c.kvstore.Set(mappingKey, []byte(roomID)); err != nil { - c.client.Log.Error("Failed to save channel mapping", "error", err, "channel_id", args.ChannelId, "room_id", roomID) + mappingValue, mErr := kvstore.BuildSingleChannelMapping(serverID, roomID) + if mErr == nil { + mErr = c.kvstore.Set(mappingKey, mappingValue) + } + if mErr != nil { + c.client.Log.Error("Failed to save channel mapping", "error", mErr, "channel_id", args.ChannelId, "room_id", roomID) return &model.CommandResponse{ ResponseType: model.CommandResponseTypeEphemeral, Text: fmt.Sprintf("✅ **Matrix Room Created:** `%s`\n\n❌ Failed to save channel mapping. Use `/matrix map %s` to map manually.", roomID, roomID), } } - // Store reverse mapping: room_mapping_ -> channelID - roomMappingKey := kvstore.BuildRoomMappingKey(roomID) + // Store reverse mapping: room_mapping__ -> channelID + roomMappingKey := kvstore.BuildRoomMappingKey(serverID, roomID) if err := c.kvstore.Set(roomMappingKey, []byte(args.ChannelId)); err != nil { c.client.Log.Error("Failed to save room mapping", "error", err, "room_id", roomID, "channel_id", args.ChannelId) // Continue anyway - the forward mapping was saved successfully @@ -672,6 +701,7 @@ func (c *Handler) executeListMappingsCommand(args *model.CommandArgs) *model.Com responseText.WriteString("**Channel-to-Room Mappings:**\n\n") // Get channel mapping keys using efficient prefix filtering + serverID := c.plugin.GetServerID() mappings := make(map[string]string) channelMappingPrefix := kvstore.KeyPrefixChannelMapping page := 0 @@ -696,8 +726,16 @@ func (c *Handler) executeListMappingsCommand(args *model.CommandArgs) *model.Com for _, key := range keys { channelID := strings.TrimPrefix(key, channelMappingPrefix) roomIDBytes, err := c.kvstore.Get(key) - if err == nil && len(roomIDBytes) > 0 { - mappings[channelID] = string(roomIDBytes) + if err != nil || len(roomIDBytes) == 0 { + continue + } + channelMappings, parseErr := kvstore.ParseChannelServerMappings(roomIDBytes) + if parseErr != nil { + c.client.Log.Warn("Failed to parse channel mapping value", "channel_id", channelID, "error", parseErr) + continue + } + if roomID := kvstore.RoomIDForServer(channelMappings, serverID); roomID != "" { + mappings[channelID] = roomID } } diff --git a/server/command/command_test.go b/server/command/command_test.go index 21add09..d30080a 100644 --- a/server/command/command_test.go +++ b/server/command/command_test.go @@ -1,6 +1,7 @@ package command import ( + "sort" "strings" "testing" @@ -9,6 +10,8 @@ import ( "github.com/mattermost/mattermost/server/public/plugin/plugintest" "github.com/mattermost/mattermost/server/public/pluginapi" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" "github.com/mattermost/mattermost-plugin-matrix-bridge/server/matrix" "github.com/mattermost/mattermost-plugin-matrix-bridge/server/store/kvstore" @@ -31,10 +34,6 @@ func (m *mockConfiguration) GetMatrixServerName() string { return "" // No configured server name in tests } -func (m *mockConfiguration) GetMatrixUsernamePrefixForServer(_ string) string { - return "matrix" // Use default prefix for tests -} - // mockPlugin implements the PluginAccessor interface for testing type mockPlugin struct { client *pluginapi.Client @@ -73,6 +72,10 @@ func (m *mockPlugin) GetRemoteID() string { return "test-remote-id" } +func (m *mockPlugin) GetServerID() string { + return "test-server-id" +} + func (m *mockPlugin) RunKVStoreMigrations() error { return nil // Mock implementation always succeeds } @@ -612,3 +615,113 @@ func TestChannelNameFallback(t *testing.T) { assert.Equal("", capturedRoomName) } } + +// memKV is a minimal in-memory kvstore.KVStore for exercising command handlers +// directly (the main package's MemoryKVStore is not importable here). +type memKV struct{ m map[string][]byte } + +func newMemKV() *memKV { return &memKV{m: map[string][]byte{}} } + +func (s *memKV) GetTemplateData(string) (string, error) { return "", nil } +func (s *memKV) Get(k string) ([]byte, error) { return s.m[k], nil } +func (s *memKV) Set(k string, v []byte) error { s.m[k] = v; return nil } +func (s *memKV) Delete(k string) error { delete(s.m, k); return nil } +func (s *memKV) ListKeys(int, int) ([]string, error) { return nil, nil } + +func (s *memKV) ListKeysWithPrefix(page, perPage int, prefix string) ([]string, error) { + var keys []string + for k := range s.m { + if strings.HasPrefix(k, prefix) { + keys = append(keys, k) + } + } + sort.Strings(keys) + start := page * perPage + if start >= len(keys) { + return nil, nil + } + end := min(start+perPage, len(keys)) + return keys[start:end], nil +} + +func newUnmapTestHandler(env *env, store kvstore.KVStore) *Handler { + return &Handler{ + plugin: &mockPlugin{ + client: env.client, + kvstore: store, + config: &mockConfiguration{serverURL: "http://test.com"}, + pluginAPI: env.api, + }, + client: env.client, + kvstore: store, + pluginAPI: env.api, + } +} + +func TestExecuteUnmapCommand(t *testing.T) { + // Note: mockPlugin.GetServerID() returns "test-server-id". + + t.Run("ClearsCorruptMapping", func(t *testing.T) { + env := setupTest() + env.api.On("GetChannel", "chanX").Return(&model.Channel{Id: "chanX", Name: "chanx"}, nil) + env.api.On("LogError", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return().Maybe() + + store := newMemKV() + key := kvstore.BuildChannelMappingKey("chanX") + require.NoError(t, store.Set(key, []byte("!not-json:server"))) + + h := newUnmapTestHandler(env, store) + resp := h.executeUnmapCommand(&model.CommandArgs{ChannelId: "chanX"}) + + assert.Contains(t, resp.Text, "Corrupt Mapping Cleared") + v, _ := store.Get(key) + assert.Empty(t, v, "corrupt mapping should be deleted so the admin can recover") + }) + + t.Run("OtherServerMappingTreatedAsUnmappedAndNotDeleted", func(t *testing.T) { + env := setupTest() + env.api.On("GetChannel", "chanY").Return(&model.Channel{Id: "chanY", Name: "chany"}, nil) + + store := newMemKV() + key := kvstore.BuildChannelMappingKey("chanY") + val, err := kvstore.BuildSingleChannelMapping("some-other-server", "!room:server") + require.NoError(t, err) + require.NoError(t, store.Set(key, val)) + + h := newUnmapTestHandler(env, store) + resp := h.executeUnmapCommand(&model.CommandArgs{ChannelId: "chanY"}) + + assert.Contains(t, resp.Text, "No Mapping Found") + v, _ := store.Get(key) + assert.NotEmpty(t, v, "a valid mapping for another server must not be deleted") + }) +} + +func TestExecuteListMappingsCommand(t *testing.T) { + // mockPlugin.GetServerID() returns "test-server-id"; only mappings for that + // server must be listed. + env := setupTest() + env.api.On("GetChannel", "chanA").Return(&model.Channel{Id: "chanA", Name: "chan-a"}, nil).Maybe() + env.api.On("GetChannel", mock.AnythingOfType("string")).Return(&model.Channel{Id: "x", Name: "x"}, nil).Maybe() + env.api.On("LogWarn", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return().Maybe() + + store := newMemKV() + // Mapping for THIS server. + valA, err := kvstore.BuildSingleChannelMapping("test-server-id", "!roomA:server") + require.NoError(t, err) + require.NoError(t, store.Set(kvstore.BuildChannelMappingKey("chanA"), valA)) + // Mapping for a DIFFERENT server — must be filtered out. + valB, err := kvstore.BuildSingleChannelMapping("other-server", "!roomB:server") + require.NoError(t, err) + require.NoError(t, store.Set(kvstore.BuildChannelMappingKey("chanB"), valB)) + // A corrupt value — must be skipped without aborting the listing. + require.NoError(t, store.Set(kvstore.BuildChannelMappingKey("chanC"), []byte("!corrupt"))) + + h := newUnmapTestHandler(env, store) + resp := h.executeListMappingsCommand(&model.CommandArgs{ChannelId: "chanZ"}) + + assert.Contains(t, resp.Text, "!roomA:server", "this server's mapping should be listed") + assert.Contains(t, resp.Text, "1 total", "only the matching-server mapping should be counted") + assert.NotContains(t, resp.Text, "!roomB:server", "another server's mapping must not be listed") + assert.NotContains(t, resp.Text, "chanC", "a corrupt mapping must be skipped, not crash the listing") +} diff --git a/server/configuration.go b/server/configuration.go index bef31a9..1e81814 100644 --- a/server/configuration.go +++ b/server/configuration.go @@ -101,7 +101,9 @@ func (p *Plugin) OnConfigurationChange() error { p.setConfiguration(configuration) - p.initMatrixClient() + if err := p.initMatrixClient(); err != nil { + return errors.Wrap(err, "failed to initialize Matrix client") + } return nil } @@ -154,11 +156,3 @@ func (c *configuration) GetMatrixUsernamePrefix() string { } return c.MatrixUsernamePrefix } - -// GetMatrixUsernamePrefixForServer returns the username prefix for a specific Matrix server -// This allows for future extensibility to support different prefixes per server -func (c *configuration) GetMatrixUsernamePrefixForServer(_ string) string { - // For now, return the global prefix - // In the future, this could check a map of server-specific prefixes - return c.GetMatrixUsernamePrefix() -} diff --git a/server/dm_room_creation_test.go b/server/dm_room_creation_test.go index 14793b0..cc33331 100644 --- a/server/dm_room_creation_test.go +++ b/server/dm_room_creation_test.go @@ -11,6 +11,7 @@ import ( "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" + "github.com/mattermost/mattermost-plugin-matrix-bridge/server/store/kvstore" matrixtest "github.com/mattermost/mattermost-plugin-matrix-bridge/testcontainers/matrix" ) @@ -104,13 +105,13 @@ func (suite *DMRoomCreationTestSuite) TestDMRoomCreationWithCorrectName() { plugin.postTracker = NewPostTracker(DefaultPostTrackerMaxEntries) // Initialize Matrix client - plugin.matrixClient = createMatrixClientWithTestLogger( + setTestMatrixClient(plugin, createMatrixClientWithTestLogger( suite.T(), suite.matrixContainer.ServerURL, suite.matrixContainer.ASToken, plugin.remoteID, - ) - plugin.matrixClient.SetServerDomain(suite.matrixContainer.ServerDomain) + )) + plugin.GetMatrixClient().SetServerDomain(suite.matrixContainer.ServerDomain) // Set up configuration config := &configuration{ @@ -124,7 +125,7 @@ func (suite *DMRoomCreationTestSuite) TestDMRoomCreationWithCorrectName() { plugin.initBridges() // Store reverse mapping for the Matrix user (simulating existing mapping) - err := plugin.kvstore.Set("mattermost_user_"+matrixUserID, []byte("@alice:"+suite.matrixContainer.ServerDomain)) + err := plugin.kvstore.Set(kvstore.BuildMattermostUserKey(testServerID, matrixUserID), []byte("@alice:"+suite.matrixContainer.ServerDomain)) require.NoError(suite.T(), err) // Create a test post from the Mattermost user to the Matrix user in the DM channel @@ -245,13 +246,13 @@ func (suite *DMRoomCreationTestSuite) TestDMRoomCreationWithMultipleUsers() { plugin.postTracker = NewPostTracker(DefaultPostTrackerMaxEntries) // Initialize Matrix client - plugin.matrixClient = createMatrixClientWithTestLogger( + setTestMatrixClient(plugin, createMatrixClientWithTestLogger( suite.T(), suite.matrixContainer.ServerURL, suite.matrixContainer.ASToken, plugin.remoteID, - ) - plugin.matrixClient.SetServerDomain(suite.matrixContainer.ServerDomain) + )) + plugin.GetMatrixClient().SetServerDomain(suite.matrixContainer.ServerDomain) // Set up configuration config := &configuration{ @@ -265,7 +266,7 @@ func (suite *DMRoomCreationTestSuite) TestDMRoomCreationWithMultipleUsers() { plugin.initBridges() // Store reverse mapping for the Matrix user - err := plugin.kvstore.Set("mattermost_user_"+matrixUserID, []byte("@alice:"+suite.matrixContainer.ServerDomain)) + err := plugin.kvstore.Set(kvstore.BuildMattermostUserKey(testServerID, matrixUserID), []byte("@alice:"+suite.matrixContainer.ServerDomain)) require.NoError(suite.T(), err) // Create a test post from the first Mattermost user in the group DM @@ -370,13 +371,13 @@ func (suite *DMRoomCreationTestSuite) TestDMRoomCreationFallbackName() { plugin.postTracker = NewPostTracker(DefaultPostTrackerMaxEntries) // Initialize Matrix client - plugin.matrixClient = createMatrixClientWithTestLogger( + setTestMatrixClient(plugin, createMatrixClientWithTestLogger( suite.T(), suite.matrixContainer.ServerURL, suite.matrixContainer.ASToken, plugin.remoteID, - ) - plugin.matrixClient.SetServerDomain(suite.matrixContainer.ServerDomain) + )) + plugin.GetMatrixClient().SetServerDomain(suite.matrixContainer.ServerDomain) // Set up configuration config := &configuration{ @@ -390,7 +391,7 @@ func (suite *DMRoomCreationTestSuite) TestDMRoomCreationFallbackName() { plugin.initBridges() // Store reverse mapping for the Matrix user - err := plugin.kvstore.Set("mattermost_user_"+matrixUserID, []byte("@alice:"+suite.matrixContainer.ServerDomain)) + err := plugin.kvstore.Set(kvstore.BuildMattermostUserKey(testServerID, matrixUserID), []byte("@alice:"+suite.matrixContainer.ServerDomain)) require.NoError(suite.T(), err) // Create a test post from the (unavailable) Mattermost user diff --git a/server/dm_support_test.go b/server/dm_support_test.go index fdc87fa..04f574b 100644 --- a/server/dm_support_test.go +++ b/server/dm_support_test.go @@ -6,6 +6,8 @@ import ( "github.com/mattermost/mattermost/server/public/model" "github.com/mattermost/mattermost/server/public/plugin/plugintest" "github.com/stretchr/testify/assert" + + "github.com/mattermost/mattermost-plugin-matrix-bridge/server/store/kvstore" ) func TestDMChannelDetection(t *testing.T) { @@ -16,8 +18,7 @@ func TestDMChannelDetection(t *testing.T) { plugin.maxFileSize = DefaultMaxFileSize plugin.postTracker = NewPostTracker(DefaultPostTrackerMaxEntries) plugin.pendingFiles = NewPendingFileTracker() - plugin.matrixClient = createMatrixClientWithTestLogger(t, "", "", "") - + setTestMatrixClient(plugin, createMatrixClientWithTestLogger(t, "", "", "")) // Initialize bridges for testing plugin.initBridges() @@ -111,7 +112,7 @@ func TestDMRoomMapping(t *testing.T) { plugin.maxFileSize = DefaultMaxFileSize plugin.postTracker = NewPostTracker(DefaultPostTrackerMaxEntries) plugin.pendingFiles = NewPendingFileTracker() - plugin.matrixClient = createMatrixClientWithTestLogger(t, "", "", "") + setTestMatrixClient(plugin, createMatrixClientWithTestLogger(t, "", "", "")) plugin.kvstore = NewMemoryKVStore() // Initialize KV store for tests // Initialize bridges for testing @@ -131,7 +132,7 @@ func TestDMRoomMapping(t *testing.T) { assert.Equal(t, matrixRoomID, retrievedRoomID) // Test reverse mapping (Matrix -> Mattermost) - roomMappingKey := "room_mapping_" + matrixRoomID + roomMappingKey := kvstore.BuildRoomMappingKey(testServerID, matrixRoomID) channelIDBytes, err := plugin.kvstore.Get(roomMappingKey) assert.NoError(t, err) assert.Equal(t, channelID, string(channelIDBytes)) diff --git a/server/hooks.go b/server/hooks.go index 173040d..63eca41 100644 --- a/server/hooks.go +++ b/server/hooks.go @@ -12,7 +12,7 @@ func (p *Plugin) OnSharedChannelsSyncMsg(msg *model.SyncMsg, _ *model.RemoteClus return model.SyncResponse{}, nil } - if p.matrixClient == nil { + if p.GetMatrixClient() == nil { p.logger.LogError("Matrix client not initialized") return model.SyncResponse{}, errors.New("matrix client not initialized") } @@ -73,14 +73,15 @@ func (p *Plugin) OnSharedChannelsPing(_ *model.RemoteCluster) bool { } // If Matrix client is not configured, we're not healthy - if p.matrixClient == nil { + matrixClient := p.GetMatrixClient() + if matrixClient == nil { p.logger.LogWarn("Ping failed - Matrix client not initialized") return false } // Test Matrix connection health if config.MatrixServerURL != "" && config.MatrixASToken != "" { - if err := p.matrixClient.TestConnection(); err != nil { + if err := matrixClient.TestConnection(); err != nil { p.logger.LogWarn("Ping failed - Matrix connection test failed", "error", err) return false } @@ -100,7 +101,8 @@ func (p *Plugin) OnSharedChannelsAttachmentSyncMsg(fi *model.FileInfo, post *mod return nil } - if p.matrixClient == nil { + matrixClient := p.GetMatrixClient() + if matrixClient == nil { return errors.New("matrix client not initialized") } @@ -134,7 +136,7 @@ func (p *Plugin) OnSharedChannelsAttachmentSyncMsg(fi *model.FileInfo, post *mod } // Upload file to Matrix but don't post it yet - just store the mxc:// URI - mxcURI, err := p.matrixClient.UploadMedia(fileData, fi.Name, fi.MimeType) + mxcURI, err := matrixClient.UploadMedia(fileData, fi.Name, fi.MimeType) if err != nil { return errors.Wrap(err, "failed to upload file to Matrix") } @@ -176,7 +178,7 @@ func (p *Plugin) deleteFileFromMatrix(fi *model.FileInfo, post *model.Post) erro } // Resolve room alias to room ID if needed - matrixRoomID, err := p.matrixClient.ResolveRoomAlias(matrixRoomIdentifier) + matrixRoomID, err := p.GetMatrixClient().ResolveRoomAlias(matrixRoomIdentifier) if err != nil { return errors.Wrap(err, "failed to resolve Matrix room identifier for file deletion") } @@ -243,14 +245,15 @@ func (p *Plugin) inviteRemoteUserToMatrixRoom(user *model.User, channelID string } // Resolve room alias to room ID (handles both aliases and room IDs) - resolvedRoomID, err := p.matrixClient.ResolveRoomAlias(matrixRoomID) + matrixClient := p.GetMatrixClient() + resolvedRoomID, err := matrixClient.ResolveRoomAlias(matrixRoomID) if err != nil { p.logger.LogWarn("Failed to resolve Matrix room identifier", "error", err, "room_identifier", matrixRoomID) return errors.Wrap(err, "failed to resolve Matrix room identifier") } // Invite the original Matrix user to the room - if err := p.matrixClient.InviteUserToRoom(resolvedRoomID, originalMatrixUserID); err != nil { + if err := matrixClient.InviteUserToRoom(resolvedRoomID, originalMatrixUserID); err != nil { p.logger.LogWarn("Failed to invite Matrix user to room", "error", err, "matrix_user_id", originalMatrixUserID, "room_id", resolvedRoomID, "mattermost_user_id", user.Id) return errors.Wrap(err, "failed to invite Matrix user to room") } @@ -266,7 +269,8 @@ func (p *Plugin) OnSharedChannelsProfileImageSyncMsg(user *model.User, _ *model. return nil } - if p.matrixClient == nil { + matrixClient := p.GetMatrixClient() + if matrixClient == nil { return errors.New("matrix client not initialized") } @@ -299,7 +303,7 @@ func (p *Plugin) OnSharedChannelsProfileImageSyncMsg(user *model.User, _ *model. } // Update the avatar for the ghost user (upload and set) - err := p.matrixClient.UpdateGhostUserAvatar(ghostUserID, avatarData, "image/png") + err := matrixClient.UpdateGhostUserAvatar(ghostUserID, avatarData, "image/png") if err != nil { p.logger.LogError("Failed to update ghost user avatar", "error", err, "user_id", user.Id, "ghost_user_id", ghostUserID) return errors.Wrap(err, "failed to update ghost user avatar on Matrix") diff --git a/server/matrix_mentions_integration_test.go b/server/matrix_mentions_integration_test.go index b36d83f..289828d 100644 --- a/server/matrix_mentions_integration_test.go +++ b/server/matrix_mentions_integration_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/mattermost/mattermost-plugin-matrix-bridge/server/store/kvstore" matrixtest "github.com/mattermost/mattermost-plugin-matrix-bridge/testcontainers/matrix" ) @@ -93,7 +94,8 @@ func TestMatrixMentionProcessing(t *testing.T) { freshRoomID := matrixContainer.CreateRoom(t, "Mention Room - "+tc.name) // Update KV store mapping for this fresh room - _ = setup.Plugin.kvstore.Set("channel_mapping_"+setup.ChannelID, []byte(freshRoomID)) + mv, _ := kvstore.BuildSingleChannelMapping(testServerID, freshRoomID) + _ = setup.Plugin.kvstore.Set(kvstore.BuildChannelMappingKey(setup.ChannelID), mv) // Clear previous mock expectations clearMockExpectations(setup.API) @@ -293,7 +295,8 @@ func TestMatrixMentionEdgeCases(t *testing.T) { freshRoomID := matrixContainer.CreateRoom(t, "Edge Case Room - "+tc.name) // Update KV store mapping for this fresh room - _ = setup.Plugin.kvstore.Set("channel_mapping_"+setup.ChannelID, []byte(freshRoomID)) + mv, _ := kvstore.BuildSingleChannelMapping(testServerID, freshRoomID) + _ = setup.Plugin.kvstore.Set(kvstore.BuildChannelMappingKey(setup.ChannelID), mv) // Clear previous mock expectations clearMockExpectations(setup.API) diff --git a/server/matrix_util.go b/server/matrix_util.go index 2809073..49d60af 100644 --- a/server/matrix_util.go +++ b/server/matrix_util.go @@ -7,11 +7,13 @@ import ( md "github.com/JohannesKaufmann/html-to-markdown" "github.com/pkg/errors" + + "github.com/mattermost/mattermost-plugin-matrix-bridge/server/store/kvstore" ) // getGhostUser retrieves the Matrix ghost user ID for a Mattermost user if it exists func (p *Plugin) getGhostUser(mattermostUserID string) (string, bool) { - ghostUserKey := "ghost_user_" + mattermostUserID + ghostUserKey := kvstore.BuildGhostUserKey(p.getSingleServerID(), mattermostUserID) ghostUserIDBytes, err := p.kvstore.Get(ghostUserKey) if err == nil && len(ghostUserIDBytes) > 0 { return string(ghostUserIDBytes), true @@ -43,8 +45,9 @@ func extractServerDomain(logger Logger, serverURL string) string { // findAndDeleteFileMessage finds and deletes file attachment messages that are replies to the main post func (p *Plugin) findAndDeleteFileMessage(matrixRoomID, ghostUserID, filename, postEventID string) error { + matrixClient := p.GetMatrixClient() // Get all reply messages to the main post event - relations, err := p.matrixClient.GetEventRelationsAsUser(matrixRoomID, postEventID, ghostUserID) + relations, err := matrixClient.GetEventRelationsAsUser(matrixRoomID, postEventID, ghostUserID) if err != nil { return errors.Wrap(err, "failed to get event relations from Matrix") } @@ -114,7 +117,7 @@ func (p *Plugin) findAndDeleteFileMessage(matrixRoomID, ghostUserID, filename, p } // Redact the file message - _, err = p.matrixClient.RedactEventAsGhost(matrixRoomID, fileEventID, ghostUserID) + _, err = matrixClient.RedactEventAsGhost(matrixRoomID, fileEventID, ghostUserID) if err != nil { return errors.Wrap(err, "failed to redact file message in Matrix") } diff --git a/server/matrix_webhook.go b/server/matrix_webhook.go index ae4c894..a657c7c 100644 --- a/server/matrix_webhook.go +++ b/server/matrix_webhook.go @@ -220,7 +220,7 @@ func (p *Plugin) processMatrixEvent(event MatrixEvent) error { // getChannelIDFromMatrixRoom finds the Mattermost channel ID for a Matrix room ID func (p *Plugin) getChannelIDFromMatrixRoom(roomID string) (string, error) { // First check KV store mapping (trusted source): room_mapping_ -> channelID - roomMappingKey := kvstore.BuildRoomMappingKey(roomID) + roomMappingKey := kvstore.BuildRoomMappingKey(p.getSingleServerID(), roomID) channelIDBytes, err := p.kvstore.Get(roomMappingKey) if err == nil && len(channelIDBytes) > 0 { channelID := string(channelIDBytes) @@ -229,8 +229,9 @@ func (p *Plugin) getChannelIDFromMatrixRoom(roomID string) (string, error) { } // Fallback: get channel ID from Matrix room state (for race condition during room creation) - if p.matrixClient != nil { - channelID, err := p.matrixClient.GetMattermostChannelID(roomID) + matrixClient := p.GetMatrixClient() + if matrixClient != nil { + channelID, err := matrixClient.GetMattermostChannelID(roomID) if err != nil { p.logger.LogDebug("Failed to get channel ID from room state", "room_id", roomID, "error", err) } else if channelID != "" { @@ -345,7 +346,7 @@ func (p *Plugin) createDMChannelForGhostUser(roomID, ghostUserID, matrixUserID s } // Verify that this ghost user exists in our KV store (meaning we created it) - ghostUserKey := kvstore.BuildGhostUserKey(mattermostUserID) + ghostUserKey := kvstore.BuildGhostUserKey(p.getSingleServerID(), mattermostUserID) ghostUserData, err := p.kvstore.Get(ghostUserKey) if err != nil || len(ghostUserData) == 0 { p.logger.LogDebug("Rejecting DM creation for unrecognized ghost user", "ghost_user_id", ghostUserID, "mattermost_user_id", mattermostUserID) @@ -370,7 +371,8 @@ func (p *Plugin) createDMChannelForGhostUser(roomID, ghostUserID, matrixUserID s Logger: p.logger, API: p.API, KVStore: p.kvstore, - MatrixClient: p.matrixClient, + MatrixClient: p.GetMatrixClient(), + ServerID: p.getSingleServerID(), RemoteID: p.remoteID, ConfigGetter: p, }) diff --git a/server/migrations.go b/server/migrations.go index ea1cbee..6a233e0 100644 --- a/server/migrations.go +++ b/server/migrations.go @@ -68,6 +68,14 @@ func (p *Plugin) runKVStoreMigrationsWithResults() (*MigrationResult, error) { result.ReverseDMMappingsCreated += v2Result.ReverseDMMappingsCreated } + if currentVersion < 3 { + v3Result, err := p.runMigrationToVersion3WithResults() + if err != nil { + return nil, errors.Wrap(err, "failed to migrate to version 3") + } + result.ChannelMappingsCreated += v3Result.ChannelMappingsCreated + } + // Update version marker if err := p.kvstore.Set(kvstore.KeyStoreVersion, []byte(strconv.Itoa(kvstore.CurrentKVStoreVersion))); err != nil { return nil, errors.Wrap(err, "failed to update KV store version") @@ -110,6 +118,9 @@ func (p *Plugin) migrateUserMappingsWithResults() (*MigrationResult, error) { p.logger.LogInfo("Migrating user mappings to add reverse lookups") userMappingPrefix := kvstore.KeyPrefixMatrixUser + // Keys already namespaced by v3 must be skipped so this legacy repair is a + // no-op when re-run over migrated data (e.g. via the /matrix migrate command). + namespace := p.serverIDNamespace() totalMigratedCount := 0 page := 0 @@ -142,8 +153,18 @@ func (p *Plugin) migrateUserMappingsWithResults() (*MigrationResult, error) { mattermostUserID := string(mattermostUserIDBytes) matrixUserID := strings.TrimPrefix(key, userMappingPrefix) - // Create reverse mapping: mattermost_user_ -> matrixUserID - reverseKey := kvstore.BuildMattermostUserKey(mattermostUserID) + // Skip keys already namespaced by v3; deriving a reverse mapping from + // them would embed the serverID in the value and corrupt the record. + if namespace != "" && strings.HasPrefix(matrixUserID, namespace) { + continue + } + + // Create reverse mapping: mattermost_user_ -> matrixUserID. + // v1/v2 reconstruct the historical (un-namespaced) key layout; the v3 + // migration is the single authority that namespaces legacy keys by + // serverID. (New runtime writes are namespaced directly by the key + // builders in the kvstore package.) + reverseKey := kvstore.KeyPrefixMattermostUser + mattermostUserID // Check if reverse mapping already exists with correct value existingData, err := p.kvstore.Get(reverseKey) @@ -217,11 +238,18 @@ func (p *Plugin) migrateChannelMappingsWithResults() (*MigrationResult, error) { continue } + // Skip values already converted to the v3 []ChannelServerMapping shape; + // treating that JSON as a room identifier would create a garbage reverse + // mapping. Keeps this legacy repair a no-op when re-run over migrated data. + if mappings, perr := kvstore.ParseChannelServerMappings(roomIdentifierBytes); perr == nil && len(mappings) > 0 { + continue + } + roomIdentifier := string(roomIdentifierBytes) channelID := strings.TrimPrefix(key, channelMappingPrefix) // Create reverse mapping: room_mapping_ -> channelID - reverseKey := kvstore.BuildRoomMappingKey(roomIdentifier) + reverseKey := kvstore.KeyPrefixRoomMapping + roomIdentifier // Check if reverse mapping already exists with correct value existingData, err := p.kvstore.Get(reverseKey) @@ -242,9 +270,10 @@ func (p *Plugin) migrateChannelMappingsWithResults() (*MigrationResult, error) { } // Always try room ID mapping for aliases, regardless of reverse mapping result - if strings.HasPrefix(roomIdentifier, "#") && p.matrixClient != nil { - if resolvedRoomID, resolveErr := p.matrixClient.ResolveRoomAlias(roomIdentifier); resolveErr == nil { - roomIDKey := kvstore.BuildRoomMappingKey(resolvedRoomID) + matrixClient := p.GetMatrixClient() + if strings.HasPrefix(roomIdentifier, "#") && matrixClient != nil { + if resolvedRoomID, resolveErr := matrixClient.ResolveRoomAlias(roomIdentifier); resolveErr == nil { + roomIDKey := kvstore.KeyPrefixRoomMapping + resolvedRoomID // Always update room ID mapping to match alias mapping if err := p.kvstore.Set(roomIDKey, []byte(channelID)); err != nil { @@ -344,7 +373,7 @@ func (p *Plugin) migrateDMMappingsWithResults() (*MigrationResult, error) { } // Also create reverse mapping for room_mapping_ if it doesn't exist - reverseKey := kvstore.BuildRoomMappingKey(matrixRoomID) + reverseKey := kvstore.KeyPrefixRoomMapping + matrixRoomID existingReverse, err := p.kvstore.Get(reverseKey) if err != nil || len(existingReverse) == 0 { if err := p.kvstore.Set(reverseKey, []byte(channelID)); err != nil { @@ -406,7 +435,7 @@ func (p *Plugin) migrateDMMappingsWithResults() (*MigrationResult, error) { matrixRoomID := strings.TrimPrefix(key, matrixDMMappingPrefix) // Create unified reverse mapping: room_mapping_ -> channelID - unifiedReverseKey := kvstore.BuildRoomMappingKey(matrixRoomID) + unifiedReverseKey := kvstore.KeyPrefixRoomMapping + matrixRoomID // Check if unified reverse mapping already exists existingReverseData, err := p.kvstore.Get(unifiedReverseKey) @@ -444,3 +473,194 @@ func (p *Plugin) migrateDMMappingsWithResults() (*MigrationResult, error) { p.logger.LogInfo("DM mapping migration completed", "total_migrated", totalMigratedCount, "total_reverse_migrated", totalReverseMigratedCount, "pages_processed", page+1) return &MigrationResult{DMMappingsCreated: totalMigratedCount, ReverseDMMappingsCreated: totalReverseMigratedCount}, nil } + +// Migration invariant (important for adding future versions): +// - v1/v2 always (re)produce the historical UN-namespaced key layout; they +// deliberately hand-build legacy keys (e.g. KeyPrefixMattermostUser + id) +// rather than the serverID key builders, and skip keys already namespaced. +// - v3 is the SOLE authority that adds the serverID namespace, and its rekey / +// value-conversion steps are idempotent, so re-running the whole chain (e.g. +// via the /matrix migrate command) is a no-op on already-migrated data. +// A future v4 must preserve this: legacy steps stay un-namespaced, and the +// namespacing/idempotency guards must account for the v3 layout. + +// v3NamespacedPrefixes are the per-server KV key prefixes that gain a serverID +// dimension in version 3. channel_mapping_ is intentionally excluded: its key +// stays server-agnostic and only its value shape changes (see below). +var v3NamespacedPrefixes = []string{ + kvstore.KeyPrefixMatrixUser, + kvstore.KeyPrefixMattermostUser, + kvstore.KeyPrefixGhostUser, + kvstore.KeyPrefixGhostRoom, + kvstore.KeyPrefixMatrixEventPost, + kvstore.KeyPrefixMatrixReaction, + kvstore.KeyPrefixRoomMapping, +} + +// runMigrationToVersion3WithResults migrates to version 3: namespaces every +// per-server KV key by the single server's serverID and converts each +// channel_mapping_ value from a bare room ID string into a []ChannelServerMapping +// JSON array. It is deterministic and idempotent: keys already namespaced and +// values already converted are skipped, so a direct re-run is a no-op. +func (p *Plugin) runMigrationToVersion3WithResults() (*MigrationResult, error) { + p.logger.LogInfo("Running migration to version 3: namespacing keys by serverID") + + // The server registry (and thus the serverID) is established by + // reconcileServerConfig during initMatrixClient, which runs before + // migrations. Reconcile again defensively in case migrations run first. + serverID := p.getSingleServerID() + if serverID == "" { + if _, err := p.reconcileServerConfig(); err != nil { + return nil, errors.Wrap(err, "failed to establish server registry for v3 migration") + } + serverID = p.getSingleServerID() + } + if serverID == "" { + // No Matrix server is configured yet (e.g. a fresh install with sync + // disabled and no server URL). There are no per-server keys to namespace, + // so the v3 layout is trivially satisfied. The registry entry and its + // namespaced keys are created later, once a server URL is configured and + // reconcileServerConfig derives the serverID. + p.logger.LogInfo("v3 migration: no Matrix server configured; nothing to namespace") + return &MigrationResult{}, nil + } + + result := &MigrationResult{} + + for _, prefix := range v3NamespacedPrefixes { + migrated, err := p.rekeyPrefixWithServerID(prefix, serverID) + if err != nil { + return nil, errors.Wrapf(err, "failed to namespace keys for prefix %q", prefix) + } + p.logger.LogDebug("Namespaced keys for prefix", "prefix", prefix, "migrated", migrated) + } + + converted, err := p.convertChannelMappingsToServerScoped(serverID) + if err != nil { + return nil, errors.Wrap(err, "failed to convert channel mappings") + } + result.ChannelMappingsCreated = converted + + p.logger.LogInfo("Version 3 migration completed", "server_id", serverID, "channel_mappings_converted", converted) + return result, nil +} + +// rekeyPrefixWithServerID rewrites every key under the given prefix from +// "" to "_". Existing keys are fully +// enumerated before any writes so newly namespaced keys (which share the prefix) +// are not re-processed, and keys already carrying the serverID namespace are +// skipped for idempotency. +func (p *Plugin) rekeyPrefixWithServerID(prefix, serverID string) (int, error) { + keys, err := p.listAllKeysWithPrefix(prefix) + if err != nil { + return 0, err + } + + namespace := serverID + "_" + migrated := 0 + failures := 0 + for _, oldKey := range keys { + id := strings.TrimPrefix(oldKey, prefix) + if strings.HasPrefix(id, namespace) { + continue // already namespaced (idempotent re-run) + } + + value, err := p.kvstore.Get(oldKey) + if err != nil { + p.logger.LogWarn("Failed to read key during v3 namespacing", "key", oldKey, "error", err) + failures++ + continue + } + + newKey := prefix + namespace + id + if err := p.kvstore.Set(newKey, value); err != nil { + p.logger.LogWarn("Failed to write namespaced key during v3 migration", "key", newKey, "error", err) + failures++ + continue + } + // A failed Delete only leaves an orphaned legacy key; the namespaced key + // is written, so reads succeed. Treat it as non-fatal. + if err := p.kvstore.Delete(oldKey); err != nil { + p.logger.LogWarn("Failed to delete legacy key during v3 migration", "key", oldKey, "error", err) + } + migrated++ + } + + if failures > 0 { + // Return an error so the version marker is not advanced to 3 and the + // migration retries on the next activation. Otherwise a transient KV + // failure would leave some mappings in the legacy namespace forever. + return migrated, errors.Errorf("failed to namespace %d key(s) for prefix %q", failures, prefix) + } + return migrated, nil +} + +// convertChannelMappingsToServerScoped rewrites each channel_mapping_ value from +// a bare room-ID string into a single-entry []ChannelServerMapping JSON array +// attributed to serverID. Values already stored as a non-empty JSON array are +// left untouched, making the conversion idempotent. +func (p *Plugin) convertChannelMappingsToServerScoped(serverID string) (int, error) { + keys, err := p.listAllKeysWithPrefix(kvstore.KeyPrefixChannelMapping) + if err != nil { + return 0, err + } + + converted := 0 + failures := 0 + for _, key := range keys { + value, err := p.kvstore.Get(key) + if err != nil { + p.logger.LogWarn("Failed to read channel mapping during v3 migration", "key", key, "error", err) + failures++ + continue + } + if len(value) == 0 { + continue + } + + // Skip values already in the new []ChannelServerMapping shape. + if existing, perr := kvstore.ParseChannelServerMappings(value); perr == nil && len(existing) > 0 { + continue + } + + newValue, err := kvstore.BuildSingleChannelMapping(serverID, string(value)) + if err != nil { + p.logger.LogWarn("Failed to marshal channel mapping during v3 migration", "key", key, "error", err) + failures++ + continue + } + if err := p.kvstore.Set(key, newValue); err != nil { + p.logger.LogWarn("Failed to write converted channel mapping during v3 migration", "key", key, "error", err) + failures++ + continue + } + converted++ + } + + if failures > 0 { + // Do not let the version marker advance to 3 with un-converted values, + // which GetMatrixRoomID would then read as unmapped. Retry next activation. + return converted, errors.Errorf("failed to convert %d channel mapping(s)", failures) + } + return converted, nil +} + +// listAllKeysWithPrefix enumerates every key for a prefix by paging through the +// KV store. The full list is materialized so callers can safely mutate keys +// sharing the prefix without disturbing pagination. +func (p *Plugin) listAllKeysWithPrefix(prefix string) ([]string, error) { + var keys []string + page := 0 + for { + batch, err := p.kvstore.ListKeysWithPrefix(page, MigrationBatchSize, prefix) + if err != nil { + return nil, errors.Wrap(err, "failed to list KV store keys with prefix") + } + keys = append(keys, batch...) + if len(batch) < MigrationBatchSize { + break + } + page++ + } + return keys, nil +} diff --git a/server/migrations_test.go b/server/migrations_test.go index eb71a17..5af01d2 100644 --- a/server/migrations_test.go +++ b/server/migrations_test.go @@ -2,13 +2,44 @@ package main import ( "strconv" + "strings" "testing" + "github.com/pkg/errors" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/mattermost/mattermost-plugin-matrix-bridge/server/store/kvstore" ) +// failOnSetKVStore wraps a KVStore and returns an error from Set for any key +// containing failKeySubstr, to simulate a transient KV backend failure. +type failOnSetKVStore struct { + kvstore.KVStore + failKeySubstr string +} + +func (f *failOnSetKVStore) Set(key string, value []byte) error { + if strings.Contains(key, f.failKeySubstr) { + return errors.New("simulated KV set failure") + } + return f.KVStore.Set(key, value) +} + +// failOnGetKVStore wraps a KVStore and returns an error from Get for any key +// containing failKeySubstr, to simulate a transient KV backend read failure. +type failOnGetKVStore struct { + kvstore.KVStore + failKeySubstr string +} + +func (f *failOnGetKVStore) Get(key string) ([]byte, error) { + if strings.Contains(key, f.failKeySubstr) { + return nil, errors.New("simulated KV get failure") + } + return f.KVStore.Get(key) +} + func TestRunKVStoreMigrations(t *testing.T) { t.Run("NoMigrationNeeded", func(t *testing.T) { plugin := setupPluginForTest() @@ -35,13 +66,15 @@ func TestRunKVStoreMigrations(t *testing.T) { plugin := setupPluginForTest() plugin.kvstore = NewMemoryKVStore() plugin.logger = &testLogger{t: t} - plugin.matrixClient = createMatrixClientWithTestLogger(t, "", "", "") + // Seed a deterministic server registry so v3 namespacing is predictable. + seedTestServerConfig(plugin) // No version key exists (version 0) - _, err := plugin.kvstore.Get(kvstore.KeyStoreVersion) - assert.Error(t, err) // Should not exist + versionData, err := plugin.kvstore.Get(kvstore.KeyStoreVersion) + require.NoError(t, err) + assert.Empty(t, versionData) // Should not exist yet - // Add some test data that would need migration + // Add some legacy (un-namespaced) test data that would need migration err = plugin.kvstore.Set("matrix_user_@alice:matrix.org", []byte("user123")) assert.NoError(t, err) err = plugin.kvstore.Set("channel_mapping_channel456", []byte("!room789:matrix.org")) @@ -58,14 +91,25 @@ func TestRunKVStoreMigrations(t *testing.T) { assert.NoError(t, err) assert.Equal(t, kvstore.CurrentKVStoreVersion, version) - // Reverse mappings should be created - userReverseBytes, err := plugin.kvstore.Get("mattermost_user_user123") + // Reverse mappings should be created and namespaced by serverID (v1 + v3) + userReverseBytes, err := plugin.kvstore.Get(kvstore.BuildMattermostUserKey(testServerID, "user123")) assert.NoError(t, err) assert.Equal(t, "@alice:matrix.org", string(userReverseBytes)) - channelReverseBytes, err := plugin.kvstore.Get("room_mapping_!room789:matrix.org") + channelReverseBytes, err := plugin.kvstore.Get(kvstore.BuildRoomMappingKey(testServerID, "!room789:matrix.org")) assert.NoError(t, err) assert.Equal(t, "channel456", string(channelReverseBytes)) + + // The forward user mapping is namespaced and the legacy key removed + userForward, err := plugin.kvstore.Get(kvstore.BuildMatrixUserKey(testServerID, "@alice:matrix.org")) + assert.NoError(t, err) + assert.Equal(t, "user123", string(userForward)) + legacyUser, err := plugin.kvstore.Get("matrix_user_@alice:matrix.org") + require.NoError(t, err) + assert.Empty(t, legacyUser) + + // The channel mapping value is converted to the []ChannelServerMapping shape + assertChannelRoom(t, plugin, "channel456", "!room789:matrix.org") }) t.Run("InvalidVersionHandledGracefully", func(t *testing.T) { @@ -114,7 +158,7 @@ func TestMigrateUserMappings(t *testing.T) { err = plugin.kvstore.Set("other_key", []byte("other_value")) assert.NoError(t, err) - // Run user migration + // Run user migration (v1 sub-step: produces legacy un-namespaced keys) _, err = plugin.migrateUserMappingsWithResults() assert.NoError(t, err) @@ -187,12 +231,12 @@ func TestMigrateUserMappings(t *testing.T) { _, err := plugin.migrateUserMappingsWithResults() assert.NoError(t, err) - // Verify all reverse mappings were created + // Verify all reverse mappings were created (legacy un-namespaced key layout) for i := range MigrationBatchSize + 100 { mattermostUserID := "user" + strconv.Itoa(i) expectedMatrixUserID := "@user" + strconv.Itoa(i) + ":matrix.org" - reverseKey := kvstore.BuildMattermostUserKey(mattermostUserID) + reverseKey := kvstore.KeyPrefixMattermostUser + mattermostUserID valueBytes, err := plugin.kvstore.Get(reverseKey) assert.NoError(t, err) assert.Equal(t, expectedMatrixUserID, string(valueBytes)) @@ -205,7 +249,7 @@ func TestMigrateChannelMappings(t *testing.T) { plugin := setupPluginForTest() plugin.kvstore = NewMemoryKVStore() plugin.logger = &testLogger{t: t} - plugin.matrixClient = createMatrixClientWithTestLogger(t, "", "", "") + setTestMatrixClient(plugin, createMatrixClientWithTestLogger(t, "", "", "")) // Add test channel mappings with room IDs testChannels := map[string]string{ @@ -218,7 +262,7 @@ func TestMigrateChannelMappings(t *testing.T) { assert.NoError(t, err) } - // Run channel migration + // Run channel migration (v1 sub-step: produces legacy un-namespaced keys) _, err := plugin.migrateChannelMappingsWithResults() assert.NoError(t, err) @@ -239,8 +283,7 @@ func TestMigrateChannelMappings(t *testing.T) { plugin := setupPluginForTest() plugin.kvstore = NewMemoryKVStore() plugin.logger = &testLogger{t: t} - // Use nil Matrix client to simulate alias resolution failure - plugin.matrixClient = nil + // No Matrix client configured, simulating alias resolution failure. // Add test channel mapping with alias err := plugin.kvstore.Set("channel_mapping_channel123", []byte("#test:matrix.org")) @@ -256,8 +299,9 @@ func TestMigrateChannelMappings(t *testing.T) { assert.Equal(t, "channel123", string(aliasReverseBytes)) // Room ID mapping should not exist due to nil client - _, err = plugin.kvstore.Get("room_mapping_!any:matrix.org") - assert.Error(t, err) + anyRoom, err := plugin.kvstore.Get("room_mapping_!any:matrix.org") + require.NoError(t, err) + assert.Empty(t, anyRoom) }) t.Run("MigrateChannelsWithAliasesAndWorkingClient", func(t *testing.T) { @@ -265,7 +309,7 @@ func TestMigrateChannelMappings(t *testing.T) { plugin.kvstore = NewMemoryKVStore() plugin.logger = &testLogger{t: t} // Use real Matrix client (though it won't actually resolve without server) - plugin.matrixClient = createMatrixClientWithTestLogger(t, "https://test.matrix.org", "test_token", "test_remote") + setTestMatrixClient(plugin, createMatrixClientWithTestLogger(t, "https://test.matrix.org", "test_token", "test_remote")) // Add test channel mapping with alias err := plugin.kvstore.Set("channel_mapping_channel123", []byte("#test:matrix.org")) @@ -311,7 +355,7 @@ func TestMigrateChannelMappings(t *testing.T) { plugin := setupPluginForTest() plugin.kvstore = NewMemoryKVStore() plugin.logger = &testLogger{t: t} - plugin.matrixClient = createMatrixClientWithTestLogger(t, "", "", "") + setTestMatrixClient(plugin, createMatrixClientWithTestLogger(t, "", "", "")) // Add more than one batch worth of channel mappings to test pagination for i := range MigrationBatchSize + 50 { @@ -325,12 +369,12 @@ func TestMigrateChannelMappings(t *testing.T) { _, err := plugin.migrateChannelMappingsWithResults() assert.NoError(t, err) - // Verify all reverse mappings were created + // Verify all reverse mappings were created (legacy un-namespaced key layout) for i := range MigrationBatchSize + 50 { channelID := "channel" + strconv.Itoa(i) roomID := "!room" + strconv.Itoa(i) + ":matrix.org" - reverseKey := kvstore.BuildRoomMappingKey(roomID) + reverseKey := kvstore.KeyPrefixRoomMapping + roomID valueBytes, err := plugin.kvstore.Get(reverseKey) assert.NoError(t, err) assert.Equal(t, channelID, string(valueBytes)) @@ -343,9 +387,9 @@ func TestMigrationIntegration(t *testing.T) { plugin := setupPluginForTest() plugin.kvstore = NewMemoryKVStore() plugin.logger = &testLogger{t: t} - plugin.matrixClient = createMatrixClientWithTestLogger(t, "", "", "") + seedTestServerConfig(plugin) - // Setup a complete scenario with users, channels, and other keys + // Setup a complete scenario with users, channels, and other keys (legacy layout) testData := map[string]string{ // User mappings "matrix_user_@alice:matrix.org": "user123", @@ -355,9 +399,11 @@ func TestMigrationIntegration(t *testing.T) { "channel_mapping_channel789": "!room012:matrix.org", "channel_mapping_channel345": "#public:matrix.org", - // Other keys (should be ignored) + // Ghost user (namespaced by v3) "ghost_user_user123": "@_mattermost_user123:matrix.org", - "some_other_key": "some_value", + + // Non per-server key (should be ignored) + "some_other_key": "some_value", } // DM mappings (will be migrated by version 2 migration) @@ -376,8 +422,9 @@ func TestMigrationIntegration(t *testing.T) { } // Verify no version key exists initially - _, err := plugin.kvstore.Get(kvstore.KeyStoreVersion) - assert.Error(t, err) + versionData, err := plugin.kvstore.Get(kvstore.KeyStoreVersion) + require.NoError(t, err) + assert.Empty(t, versionData) // Run full migration err = plugin.runKVStoreMigrations() @@ -390,57 +437,69 @@ func TestMigrationIntegration(t *testing.T) { assert.NoError(t, err) assert.Equal(t, kvstore.CurrentKVStoreVersion, version) - // Check user reverse mappings - userReverse1, err := plugin.kvstore.Get("mattermost_user_user123") + // Check user reverse mappings (namespaced by serverID) + userReverse1, err := plugin.kvstore.Get(kvstore.BuildMattermostUserKey(testServerID, "user123")) assert.NoError(t, err) assert.Equal(t, "@alice:matrix.org", string(userReverse1)) - userReverse2, err := plugin.kvstore.Get("mattermost_user_user456") + userReverse2, err := plugin.kvstore.Get(kvstore.BuildMattermostUserKey(testServerID, "user456")) assert.NoError(t, err) assert.Equal(t, "@bob:matrix.org", string(userReverse2)) - // Check channel reverse mappings - channelReverse1, err := plugin.kvstore.Get("room_mapping_!room012:matrix.org") + // Check channel reverse mappings (namespaced by serverID) + channelReverse1, err := plugin.kvstore.Get(kvstore.BuildRoomMappingKey(testServerID, "!room012:matrix.org")) assert.NoError(t, err) assert.Equal(t, "channel789", string(channelReverse1)) - channelReverse2, err := plugin.kvstore.Get("room_mapping_#public:matrix.org") + channelReverse2, err := plugin.kvstore.Get(kvstore.BuildRoomMappingKey(testServerID, "#public:matrix.org")) assert.NoError(t, err) assert.Equal(t, "channel345", string(channelReverse2)) - // Verify original data is unchanged - for key, expectedValue := range testData { - valueBytes, err := plugin.kvstore.Get(key) - assert.NoError(t, err) - assert.Equal(t, expectedValue, string(valueBytes)) - } - - // Verify DM mappings were migrated to unified prefix - dmUnifiedBytes, err := plugin.kvstore.Get("channel_mapping_dm123") + // Forward user + ghost mappings are namespaced; legacy keys removed + userForward, err := plugin.kvstore.Get(kvstore.BuildMatrixUserKey(testServerID, "@alice:matrix.org")) assert.NoError(t, err) - assert.Equal(t, "!dmroom456:matrix.org", string(dmUnifiedBytes)) + assert.Equal(t, "user123", string(userForward)) + legacyUser, err := plugin.kvstore.Get("matrix_user_@alice:matrix.org") + require.NoError(t, err) + assert.Empty(t, legacyUser) - // Verify old DM mapping was deleted - _, err = plugin.kvstore.Get("dm_mapping_dm123") - assert.Error(t, err) // Should be deleted - - // Verify reverse DM mapping was created - dmReverseBytes, err := plugin.kvstore.Get("room_mapping_!dmroom456:matrix.org") + ghostForward, err := plugin.kvstore.Get(kvstore.BuildGhostUserKey(testServerID, "user123")) assert.NoError(t, err) - assert.Equal(t, "dm123", string(dmReverseBytes)) + assert.Equal(t, "@_mattermost_user123:matrix.org", string(ghostForward)) + legacyGhost, err := plugin.kvstore.Get("ghost_user_user123") + require.NoError(t, err) + assert.Empty(t, legacyGhost) + // Channel mapping values are converted to []ChannelServerMapping + assertChannelRoom(t, plugin, "channel789", "!room012:matrix.org") + assertChannelRoom(t, plugin, "channel345", "#public:matrix.org") + + // Non per-server key is untouched otherBytes, err := plugin.kvstore.Get("some_other_key") assert.NoError(t, err) assert.Equal(t, "some_value", string(otherBytes)) + + // Verify DM mapping was migrated to unified prefix and converted + assertChannelRoom(t, plugin, "dm123", "!dmroom456:matrix.org") + + // Verify old DM mapping was deleted + oldDM, err := plugin.kvstore.Get("dm_mapping_dm123") + require.NoError(t, err) + assert.Empty(t, oldDM) // Should be deleted + + // Verify reverse DM mapping was created and namespaced + dmReverseBytes, err := plugin.kvstore.Get(kvstore.BuildRoomMappingKey(testServerID, "!dmroom456:matrix.org")) + assert.NoError(t, err) + assert.Equal(t, "dm123", string(dmReverseBytes)) }) t.Run("RunMigrationTwiceIsIdempotent", func(t *testing.T) { plugin := setupPluginForTest() plugin.kvstore = NewMemoryKVStore() plugin.logger = &testLogger{t: t} - plugin.matrixClient = createMatrixClientWithTestLogger(t, "", "", "") + seedTestServerConfig(plugin) - // Add test data + // Add legacy test data err := plugin.kvstore.Set("matrix_user_@alice:matrix.org", []byte("user123")) assert.NoError(t, err) err = plugin.kvstore.Set("channel_mapping_channel456", []byte("!room789:matrix.org")) @@ -450,12 +509,17 @@ func TestMigrationIntegration(t *testing.T) { err = plugin.runKVStoreMigrations() assert.NoError(t, err) - // Verify reverse mappings exist - userReverse, err := plugin.kvstore.Get("mattermost_user_user123") + // Capture the store size after the first run + memStore, ok := plugin.kvstore.(*MemoryKVStore) + require.True(t, ok) + sizeAfterFirst := memStore.Size() + + // Verify namespaced mappings exist + userReverse, err := plugin.kvstore.Get(kvstore.BuildMattermostUserKey(testServerID, "user123")) assert.NoError(t, err) assert.Equal(t, "@alice:matrix.org", string(userReverse)) - channelReverse, err := plugin.kvstore.Get("room_mapping_!room789:matrix.org") + channelReverse, err := plugin.kvstore.Get(kvstore.BuildRoomMappingKey(testServerID, "!room789:matrix.org")) assert.NoError(t, err) assert.Equal(t, "channel456", string(channelReverse)) @@ -463,15 +527,19 @@ func TestMigrationIntegration(t *testing.T) { err = plugin.runKVStoreMigrations() assert.NoError(t, err) - // Verify data is unchanged - userReverse2, err := plugin.kvstore.Get("mattermost_user_user123") + // Data is unchanged and no keys were added or duplicated + assert.Equal(t, sizeAfterFirst, memStore.Size()) + + userReverse2, err := plugin.kvstore.Get(kvstore.BuildMattermostUserKey(testServerID, "user123")) assert.NoError(t, err) assert.Equal(t, "@alice:matrix.org", string(userReverse2)) - channelReverse2, err := plugin.kvstore.Get("room_mapping_!room789:matrix.org") + channelReverse2, err := plugin.kvstore.Get(kvstore.BuildRoomMappingKey(testServerID, "!room789:matrix.org")) assert.NoError(t, err) assert.Equal(t, "channel456", string(channelReverse2)) + assertChannelRoom(t, plugin, "channel456", "!room789:matrix.org") + // Version should still be current versionBytes, err := plugin.kvstore.Get(kvstore.KeyStoreVersion) assert.NoError(t, err) @@ -484,7 +552,6 @@ func TestMigrationIntegration(t *testing.T) { plugin := setupPluginForTest() plugin.kvstore = NewMemoryKVStore() plugin.logger = &testLogger{t: t} - plugin.matrixClient = createMatrixClientWithTestLogger(t, "", "", "") // Run migration on empty KV store err := plugin.runKVStoreMigrations() @@ -498,3 +565,338 @@ func TestMigrationIntegration(t *testing.T) { assert.Equal(t, kvstore.CurrentKVStoreVersion, version) }) } + +func TestMigrationToVersion3(t *testing.T) { + t.Run("FreshInstallCreatesSingleServerEntry", func(t *testing.T) { + plugin := setupPluginForTest() + plugin.kvstore = NewMemoryKVStore() + plugin.logger = &testLogger{t: t} + plugin.configuration = &configuration{MatrixServerURL: "https://matrix.example.com"} + + // No prior keys at all (fresh install) + err := plugin.runKVStoreMigrations() + assert.NoError(t, err) + + versionBytes, err := plugin.kvstore.Get(kvstore.KeyStoreVersion) + assert.NoError(t, err) + version, _ := strconv.Atoi(string(versionBytes)) + assert.Equal(t, kvstore.CurrentKVStoreVersion, version) + + // servers_config holds exactly one entry, keyed by the derived serverID. + servers, err := plugin.getServers() + assert.NoError(t, err) + require.Len(t, servers, 1) + expectedID, err := deriveServerID("https://matrix.example.com") + require.NoError(t, err) + assert.Equal(t, expectedID, servers[0].ServerID) + }) + + t.Run("FreshInstallWithoutServerConfiguredIsNoOp", func(t *testing.T) { + plugin := setupPluginForTest() + plugin.kvstore = NewMemoryKVStore() + plugin.logger = &testLogger{t: t} + + // Plugin enabled but no Matrix server configured (sync disabled): the v3 + // migration must complete and bump the version without creating an entry. + err := plugin.runKVStoreMigrations() + require.NoError(t, err) + + versionBytes, err := plugin.kvstore.Get(kvstore.KeyStoreVersion) + require.NoError(t, err) + version, _ := strconv.Atoi(string(versionBytes)) + assert.Equal(t, kvstore.CurrentKVStoreVersion, version) + + servers, err := plugin.getServers() + require.NoError(t, err) + assert.Empty(t, servers, "no server configured means no registry entry") + }) + + t.Run("UpgradeFromVersion2NamespacesKeys", func(t *testing.T) { + plugin := setupPluginForTest() + plugin.kvstore = NewMemoryKVStore() + plugin.logger = &testLogger{t: t} + seedTestServerConfig(plugin) + + // Simulate a v2 install: version marker at 2 with legacy, un-namespaced keys. + require.NoError(t, plugin.kvstore.Set(kvstore.KeyStoreVersion, []byte("2"))) + require.NoError(t, plugin.kvstore.Set("matrix_user_@alice:matrix.org", []byte("user123"))) + require.NoError(t, plugin.kvstore.Set("mattermost_user_user123", []byte("@alice:matrix.org"))) + require.NoError(t, plugin.kvstore.Set("room_mapping_!room789:matrix.org", []byte("channel456"))) + require.NoError(t, plugin.kvstore.Set("ghost_user_user123", []byte("@_mattermost_user123:matrix.org"))) + // ghost_room_ is a composite key: ghost_room__. + require.NoError(t, plugin.kvstore.Set("ghost_room_user123_!room789:matrix.org", []byte("joined"))) + require.NoError(t, plugin.kvstore.Set("matrix_reaction_$evt1:matrix.org", []byte("reaction-info"))) + require.NoError(t, plugin.kvstore.Set("matrix_event_post_$evt2:matrix.org", []byte("post999"))) + require.NoError(t, plugin.kvstore.Set("channel_mapping_channel456", []byte("!room789:matrix.org"))) + + // Only v3 runs. + err := plugin.runKVStoreMigrations() + assert.NoError(t, err) + + // Every per-server key is now namespaced, and the legacy keys are gone. + namespaced := map[string]string{ + kvstore.BuildMatrixUserKey(testServerID, "@alice:matrix.org"): "user123", + kvstore.BuildMattermostUserKey(testServerID, "user123"): "@alice:matrix.org", + kvstore.BuildRoomMappingKey(testServerID, "!room789:matrix.org"): "channel456", + kvstore.BuildGhostUserKey(testServerID, "user123"): "@_mattermost_user123:matrix.org", + kvstore.BuildGhostRoomKey(testServerID, "user123", "!room789:matrix.org"): "joined", + kvstore.BuildMatrixReactionKey(testServerID, "$evt1:matrix.org"): "reaction-info", + kvstore.BuildMatrixEventPostKey(testServerID, "$evt2:matrix.org"): "post999", + } + for key, expected := range namespaced { + got, err := plugin.kvstore.Get(key) + assert.NoError(t, err, key) + assert.Equal(t, expected, string(got), key) + } + + for _, legacy := range []string{ + "matrix_user_@alice:matrix.org", + "mattermost_user_user123", + "room_mapping_!room789:matrix.org", + "ghost_user_user123", + "ghost_room_user123_!room789:matrix.org", + "matrix_reaction_$evt1:matrix.org", + "matrix_event_post_$evt2:matrix.org", + } { + legacyVal, err := plugin.kvstore.Get(legacy) + require.NoError(t, err, legacy) + assert.Empty(t, legacyVal, legacy) + } + + // The channel mapping value is converted to the server-scoped shape. + assertChannelRoom(t, plugin, "channel456", "!room789:matrix.org") + }) + + t.Run("DirectReRunIsNoOp", func(t *testing.T) { + plugin := setupPluginForTest() + plugin.kvstore = NewMemoryKVStore() + plugin.logger = &testLogger{t: t} + seedTestServerConfig(plugin) + + require.NoError(t, plugin.kvstore.Set("matrix_user_@alice:matrix.org", []byte("user123"))) + require.NoError(t, plugin.kvstore.Set("channel_mapping_channel456", []byte("!room789:matrix.org"))) + + _, err := plugin.runMigrationToVersion3WithResults() + require.NoError(t, err) + + memStore := plugin.kvstore.(*MemoryKVStore) + sizeAfterFirst := memStore.Size() + firstValue, err := plugin.kvstore.Get(kvstore.BuildMatrixUserKey(testServerID, "@alice:matrix.org")) + require.NoError(t, err) + + // Running the v3 migration again must not change anything. + _, err = plugin.runMigrationToVersion3WithResults() + require.NoError(t, err) + + assert.Equal(t, sizeAfterFirst, memStore.Size()) + secondValue, err := plugin.kvstore.Get(kvstore.BuildMatrixUserKey(testServerID, "@alice:matrix.org")) + require.NoError(t, err) + assert.Equal(t, string(firstValue), string(secondValue)) + assertChannelRoom(t, plugin, "channel456", "!room789:matrix.org") + }) + + t.Run("MigrateResetReRunIsIdempotentAndDoesNotCorruptMappings", func(t *testing.T) { + // Simulates the /matrix migrate admin command (reset version to 0 and + // re-run the whole chain) against data that has already been migrated to + // v3. The legacy v1/v2 repair must not mis-derive namespaced keys. + plugin := setupPluginForTest() + plugin.kvstore = NewMemoryKVStore() + plugin.logger = &testLogger{t: t} + seedTestServerConfig(plugin) + + // A v2 install with forward + reverse user mappings and a channel mapping. + require.NoError(t, plugin.kvstore.Set(kvstore.KeyStoreVersion, []byte("2"))) + require.NoError(t, plugin.kvstore.Set("matrix_user_@alice:matrix.org", []byte("user123"))) + require.NoError(t, plugin.kvstore.Set("mattermost_user_user123", []byte("@alice:matrix.org"))) + require.NoError(t, plugin.kvstore.Set("room_mapping_!room1:matrix.org", []byte("chan1"))) + require.NoError(t, plugin.kvstore.Set("channel_mapping_chan1", []byte("!room1:matrix.org"))) + + // Normal upgrade to v3. + require.NoError(t, plugin.runKVStoreMigrations()) + + // Reset and re-run, as /matrix migrate does. + require.NoError(t, plugin.kvstore.Set(kvstore.KeyStoreVersion, []byte("0"))) + require.NoError(t, plugin.runKVStoreMigrations()) + + // Reverse user mapping must still be the Matrix user ID, not a value with + // the serverID prefix leaked into it. + rev, err := plugin.kvstore.Get(kvstore.BuildMattermostUserKey(testServerID, "user123")) + require.NoError(t, err) + assert.Equal(t, "@alice:matrix.org", string(rev)) + + // Forward user mapping and channel mapping remain correct. + fwd, err := plugin.kvstore.Get(kvstore.BuildMatrixUserKey(testServerID, "@alice:matrix.org")) + require.NoError(t, err) + assert.Equal(t, "user123", string(fwd)) + assertChannelRoom(t, plugin, "chan1", "!room1:matrix.org") + + // Reverse room mapping stays attributed to the server, uncorrupted. + roomRev, err := plugin.kvstore.Get(kvstore.BuildRoomMappingKey(testServerID, "!room1:matrix.org")) + require.NoError(t, err) + assert.Equal(t, "chan1", string(roomRev)) + }) + + t.Run("ChannelConversionSetFailureAbortsVersionBump", func(t *testing.T) { + // Failing the channel_mapping conversion write (which runs after the + // prefix rekey) must abort the migration so the version stays at 2 and + // the un-converted value is retried next activation. + base := NewMemoryKVStore() + plugin := setupPluginForTest() + plugin.logger = &testLogger{t: t} + plugin.kvstore = base + seedTestServerConfig(plugin) + + require.NoError(t, base.Set(kvstore.KeyStoreVersion, []byte("2"))) + require.NoError(t, base.Set("channel_mapping_chan1", []byte("!room1:matrix.org"))) + + // Fail only channel_mapping writes; the prefix-rekey step (other prefixes) + // still succeeds, so the failure is isolated to the conversion step. + plugin.kvstore = &failOnSetKVStore{KVStore: base, failKeySubstr: kvstore.KeyPrefixChannelMapping} + + err := plugin.runKVStoreMigrations() + require.Error(t, err, "a failed channel conversion must fail the migration") + + versionBytes, err := base.Get(kvstore.KeyStoreVersion) + require.NoError(t, err) + assert.Equal(t, "2", string(versionBytes)) + + // The value is left un-converted (bare string), to be retried later. + value, err := base.Get("channel_mapping_chan1") + require.NoError(t, err) + assert.Equal(t, "!room1:matrix.org", string(value)) + }) + + t.Run("SetFailureAbortsVersionBumpAndPreservesLegacyKey", func(t *testing.T) { + base := NewMemoryKVStore() + plugin := setupPluginForTest() + plugin.logger = &testLogger{t: t} + plugin.kvstore = base + seedTestServerConfig(plugin) + + // v2 install with a legacy un-namespaced key. + require.NoError(t, base.Set(kvstore.KeyStoreVersion, []byte("2"))) + require.NoError(t, base.Set("matrix_user_@alice:matrix.org", []byte("user123"))) + + // Fail writes of the namespaced matrix_user key to simulate a transient + // KV failure partway through the rekey. + plugin.kvstore = &failOnSetKVStore{KVStore: base, failKeySubstr: kvstore.KeyPrefixMatrixUser + testServerID} + + err := plugin.runKVStoreMigrations() + require.Error(t, err, "a failed rekey must fail the migration") + + // The version marker must NOT advance to 3, so the migration retries later. + versionBytes, err := base.Get(kvstore.KeyStoreVersion) + require.NoError(t, err) + assert.Equal(t, "2", string(versionBytes)) + + // The legacy key must still be present (it was never deleted). + legacy, err := base.Get("matrix_user_@alice:matrix.org") + require.NoError(t, err) + assert.Equal(t, "user123", string(legacy)) + }) +} + +// TestMigrateExistingSingleServerInstall exercises the full transition of an +// existing single-server install (flat plugin.json config + legacy v2 KV data, +// no registry) into the multi-server layout: the global config is projected into +// a one-entry registry with a minted serverID, all mappings are re-attributed to +// that serverID, and the per-server username prefix resolves from the registry. +func TestMigrateExistingSingleServerInstall(t *testing.T) { + plugin := setupPluginForTest() + plugin.kvstore = NewMemoryKVStore() + plugin.logger = &testLogger{t: t} + plugin.remoteID = "remote-abc" + plugin.pendingFiles = NewPendingFileTracker() + plugin.postTracker = NewPostTracker(DefaultPostTrackerMaxEntries) + + // Existing single-server install: flat plugin.json config is the only source, + // and there is no server registry yet. + plugin.configuration = &configuration{ + MatrixServerURL: "https://matrix.example.com", + MatrixServerName: "example.com", + MatrixASToken: "as-token", + MatrixHSToken: "hs-token", + MatrixUsernamePrefix: "mxprefix", + EnableSync: true, + } + + // Legacy v2 KV data written by the single-server plugin (un-namespaced keys, + // bare channel_mapping value). + require.NoError(t, plugin.kvstore.Set(kvstore.KeyStoreVersion, []byte("2"))) + require.NoError(t, plugin.kvstore.Set("matrix_user_@alice:example.com", []byte("mmuser1"))) + require.NoError(t, plugin.kvstore.Set("mattermost_user_mmuser1", []byte("@alice:example.com"))) + require.NoError(t, plugin.kvstore.Set("ghost_user_mmuser2", []byte("@_mattermost_mmuser2:example.com"))) + require.NoError(t, plugin.kvstore.Set("room_mapping_!room1:example.com", []byte("chan1"))) + require.NoError(t, plugin.kvstore.Set("channel_mapping_chan1", []byte("!room1:example.com"))) + + // No registry exists before migration. + before, err := plugin.getServers() + require.NoError(t, err) + require.Empty(t, before) + + // Run migrations, mirroring activation: reconcile mints the serverID from the + // flat config, then v3 re-attributes the existing data to it. + require.NoError(t, plugin.runKVStoreMigrations()) + + // A single registry entry now exists, projected from the flat plugin.json. + servers, err := plugin.getServers() + require.NoError(t, err) + require.Len(t, servers, 1) + entry := servers[0] + require.NotEmpty(t, entry.ServerID) + assert.Equal(t, "https://matrix.example.com", entry.ServerURL) + assert.Equal(t, "example.com", entry.ServerName) + assert.Equal(t, "as-token", entry.ASToken) + assert.Equal(t, "hs-token", entry.HSToken) + assert.Equal(t, "mxprefix", entry.UsernamePrefix) + assert.True(t, entry.Enabled) + assert.Equal(t, "remote-abc", entry.RemoteID) + + sid := entry.ServerID + + // Version advanced to current. + versionBytes, err := plugin.kvstore.Get(kvstore.KeyStoreVersion) + require.NoError(t, err) + version, err := strconv.Atoi(string(versionBytes)) + require.NoError(t, err) + assert.Equal(t, kvstore.CurrentKVStoreVersion, version) + + // Every per-server mapping is re-keyed under the minted serverID, and the + // legacy un-namespaced keys are removed. + assertMigrated := func(newKey, legacyKey, want string) { + got, err := plugin.kvstore.Get(newKey) + require.NoError(t, err) + assert.Equal(t, want, string(got), newKey) + old, err := plugin.kvstore.Get(legacyKey) + require.NoError(t, err) + assert.Empty(t, old, legacyKey) + } + assertMigrated(kvstore.BuildMatrixUserKey(sid, "@alice:example.com"), "matrix_user_@alice:example.com", "mmuser1") + assertMigrated(kvstore.BuildMattermostUserKey(sid, "mmuser1"), "mattermost_user_mmuser1", "@alice:example.com") + assertMigrated(kvstore.BuildGhostUserKey(sid, "mmuser2"), "ghost_user_mmuser2", "@_mattermost_mmuser2:example.com") + assertMigrated(kvstore.BuildRoomMappingKey(sid, "!room1:example.com"), "room_mapping_!room1:example.com", "chan1") + + // The channel mapping value is converted to the server-scoped shape. + data, err := plugin.kvstore.Get(kvstore.BuildChannelMappingKey("chan1")) + require.NoError(t, err) + mappings, err := kvstore.ParseChannelServerMappings(data) + require.NoError(t, err) + assert.Equal(t, "!room1:example.com", kvstore.RoomIDForServer(mappings, sid)) + + // The previously-global username prefix now resolves from the per-server + // registry entry, keyed by the minted serverID. + plugin.initBridges() + assert.Equal(t, "mxprefix", plugin.mattermostToMatrixBridge.matrixUsernamePrefix()) +} + +// assertChannelRoom checks that the channel_mapping_ value for channelID is the +// server-scoped []ChannelServerMapping shape and maps to expectedRoomID for the +// test server. +func assertChannelRoom(t *testing.T, plugin *Plugin, channelID, expectedRoomID string) { + t.Helper() + data, err := plugin.kvstore.Get(kvstore.BuildChannelMappingKey(channelID)) + require.NoError(t, err) + mappings, err := kvstore.ParseChannelServerMappings(data) + require.NoError(t, err) + assert.Equal(t, expectedRoomID, kvstore.RoomIDForServer(mappings, testServerID)) +} diff --git a/server/multi_server_integration_test.go b/server/multi_server_integration_test.go new file mode 100644 index 0000000..2119253 --- /dev/null +++ b/server/multi_server_integration_test.go @@ -0,0 +1,234 @@ +package main + +import ( + "encoding/json" + "strings" + "testing" + + "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/public/plugin/plugintest" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" + + "github.com/mattermost/mattermost-plugin-matrix-bridge/server/matrix" + "github.com/mattermost/mattermost-plugin-matrix-bridge/server/store/kvstore" + matrixtest "github.com/mattermost/mattermost-plugin-matrix-bridge/testcontainers/matrix" +) + +// Deterministic serverIDs for the two-server registry. +const ( + multiServerAID = "serveraserveraserveraserv01" + multiServerBID = "serverbserverbserverbserv02" +) + +// MultiServerIntegrationTestSuite verifies the multi-server backend plumbing — +// the client registry, serverID-namespaced KV, and per-server bridge operations — +// against two independent, live Synapse homeservers. +// +// Scope note: the plugin provides the multi-server backend plumbing but not +// cross-server routing (inbound/outbound server selection is still single-server). +// These tests therefore exercise the registry and per-server isolation directly, +// not an end-to-end message flow across the two servers. +type MultiServerIntegrationTestSuite struct { + suite.Suite + containerA *matrixtest.Container + containerB *matrixtest.Container + plugin *Plugin + api *plugintest.API +} + +func (suite *MultiServerIntegrationTestSuite) SetupSuite() { + // Two independent homeservers with distinct domains (each gets its own + // dynamically-assigned port from testcontainers). + suite.containerA = matrixtest.StartMatrixContainer(suite.T(), matrixtest.MatrixTestConfig{ + ServerName: "synapse-a.local", + ASToken: "as_token_server_a", + HSToken: "hs_token_server_a", + }) + suite.containerB = matrixtest.StartMatrixContainer(suite.T(), matrixtest.MatrixTestConfig{ + ServerName: "synapse-b.local", + ASToken: "as_token_server_b", + HSToken: "hs_token_server_b", + }) + suite.containerA.Client.SetServerDomain(suite.containerA.ServerDomain) + suite.containerB.Client.SetServerDomain(suite.containerB.ServerDomain) +} + +func (suite *MultiServerIntegrationTestSuite) TearDownSuite() { + if suite.containerA != nil { + suite.containerA.Cleanup(suite.T()) + } + if suite.containerB != nil { + suite.containerB.Cleanup(suite.T()) + } +} + +// SetupTest builds a plugin whose registry holds both servers, keyed by serverID. +func (suite *MultiServerIntegrationTestSuite) SetupTest() { + suite.api = &plugintest.API{} + suite.api.On("LogDebug", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Maybe() + suite.api.On("LogInfo", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Maybe() + suite.api.On("LogWarn", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Maybe() + suite.api.On("LogError", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Maybe() + + plugin := &Plugin{remoteID: "test-remote-id"} + plugin.SetAPI(suite.api) + plugin.logger = &testLogger{t: suite.T()} + plugin.kvstore = NewMemoryKVStore() + plugin.pendingFiles = NewPendingFileTracker() + plugin.postTracker = NewPostTracker(DefaultPostTrackerMaxEntries) + plugin.maxProfileImageSize = DefaultMaxProfileImageSize + plugin.maxFileSize = DefaultMaxFileSize + + // Client registry with one client per homeserver. + plugin.matrixClients = map[string]*matrix.Client{ + multiServerAID: suite.containerA.Client, + multiServerBID: suite.containerB.Client, + } + plugin.serverID = multiServerAID // GetMatrixClient()/getSingleServerID() default + + // Registry entries mirroring what reconcileServerConfig would persist. + servers := []kvstore.ServerConfig{ + {ServerID: multiServerAID, ServerURL: suite.containerA.ServerURL, ServerName: suite.containerA.ServerDomain, UsernamePrefix: "matrixa", Enabled: true, RemoteID: plugin.remoteID}, + {ServerID: multiServerBID, ServerURL: suite.containerB.ServerURL, ServerName: suite.containerB.ServerDomain, UsernamePrefix: "matrixb", Enabled: true, RemoteID: plugin.remoteID}, + } + data, err := json.Marshal(servers) + suite.Require().NoError(err) + suite.Require().NoError(plugin.kvstore.Set(kvstore.KeyServersConfig, data)) + + suite.plugin = plugin +} + +// bridgeFor builds a MattermostToMatrixBridge scoped to a single server, as the +// plugin would when it constructs one bridge per configured server. +func (suite *MultiServerIntegrationTestSuite) bridgeFor(serverID string, client *matrix.Client) *MattermostToMatrixBridge { + utils := NewBridgeUtils(BridgeUtilsConfig{ + Logger: suite.plugin.logger, + API: suite.plugin.API, + KVStore: suite.plugin.kvstore, + MatrixClient: client, + ServerID: serverID, + RemoteID: suite.plugin.remoteID, + MaxProfileImageSize: DefaultMaxProfileImageSize, + MaxFileSize: DefaultMaxFileSize, + ConfigGetter: suite.plugin, + }) + return NewMattermostToMatrixBridge(utils, suite.plugin.pendingFiles, suite.plugin.postTracker) +} + +// TestClientRegistryRoutesToCorrectHomeserver verifies getMatrixClient returns the +// right client per serverID and that each client talks to its own homeserver. +func (suite *MultiServerIntegrationTestSuite) TestClientRegistryRoutesToCorrectHomeserver() { + t := suite.T() + + assert.Same(t, suite.containerA.Client, suite.plugin.getMatrixClient(multiServerAID)) + assert.Same(t, suite.containerB.Client, suite.plugin.getMatrixClient(multiServerBID)) + assert.Nil(t, suite.plugin.getMatrixClient("no-such-server")) + // The single-server accessor resolves the cached serverID (server A). + assert.Same(t, suite.containerA.Client, suite.plugin.GetMatrixClient()) + + // Both live homeservers are reachable through their registered clients. + assert.NoError(t, suite.plugin.getMatrixClient(multiServerAID).TestConnection()) + assert.NoError(t, suite.plugin.getMatrixClient(multiServerBID).TestConnection()) + + // A room created through each client lives on that client's homeserver, as + // shown by the server domain embedded in the returned room ID. + roomA, err := suite.plugin.getMatrixClient(multiServerAID).CreateRoom("room-on-a", "", suite.containerA.ServerDomain, true, "") + require.NoError(t, err) + roomB, err := suite.plugin.getMatrixClient(multiServerBID).CreateRoom("room-on-b", "", suite.containerB.ServerDomain, true, "") + require.NoError(t, err) + + assert.True(t, strings.HasSuffix(roomA, ":"+suite.containerA.ServerDomain), "room A %q should live on %q", roomA, suite.containerA.ServerDomain) + assert.True(t, strings.HasSuffix(roomB, ":"+suite.containerB.ServerDomain), "room B %q should live on %q", roomB, suite.containerB.ServerDomain) + assert.NotEqual(t, roomA, roomB, "each client created a room on its own homeserver") +} + +// TestKVNamespacingIsolatesServers verifies serverID-namespaced keys and the +// per-server channel_mapping value keep the two servers' data independent. +func (suite *MultiServerIntegrationTestSuite) TestKVNamespacingIsolatesServers() { + t := suite.T() + + // One Mattermost channel bridged to a different room on each server. + channelID := model.NewId() + roomA := "!roomA:" + suite.containerA.ServerDomain + roomB := "!roomB:" + suite.containerB.ServerDomain + value, err := kvstore.MarshalChannelServerMappings([]kvstore.ChannelServerMapping{ + {ServerID: multiServerAID, RoomID: roomA}, + {ServerID: multiServerBID, RoomID: roomB}, + }) + require.NoError(t, err) + require.NoError(t, suite.plugin.kvstore.Set(kvstore.BuildChannelMappingKey(channelID), value)) + + // Each per-server bridge resolves only its own server's room. + bridgeA := suite.bridgeFor(multiServerAID, suite.containerA.Client) + bridgeB := suite.bridgeFor(multiServerBID, suite.containerB.Client) + + gotA, err := bridgeA.GetMatrixRoomID(channelID) + require.NoError(t, err) + assert.Equal(t, roomA, gotA) + + gotB, err := bridgeB.GetMatrixRoomID(channelID) + require.NoError(t, err) + assert.Equal(t, roomB, gotB) + + // The same Matrix user ID maps independently under each server's namespace. + matrixUserID := "@shared:example.org" + require.NoError(t, suite.plugin.kvstore.Set(kvstore.BuildMatrixUserKey(multiServerAID, matrixUserID), []byte("mmuser-on-a"))) + require.NoError(t, suite.plugin.kvstore.Set(kvstore.BuildMatrixUserKey(multiServerBID, matrixUserID), []byte("mmuser-on-b"))) + + valA, err := suite.plugin.kvstore.Get(kvstore.BuildMatrixUserKey(multiServerAID, matrixUserID)) + require.NoError(t, err) + valB, err := suite.plugin.kvstore.Get(kvstore.BuildMatrixUserKey(multiServerBID, matrixUserID)) + require.NoError(t, err) + assert.Equal(t, "mmuser-on-a", string(valA)) + assert.Equal(t, "mmuser-on-b", string(valB)) +} + +// TestGhostUsersAreCreatedPerServer verifies that creating a ghost user for the +// same Mattermost user on each server produces distinct ghosts on the respective +// homeservers, cached under serverID-namespaced keys. +func (suite *MultiServerIntegrationTestSuite) TestGhostUsersAreCreatedPerServer() { + t := suite.T() + + mmUserID := model.NewId() + suite.api.On("GetUser", mmUserID).Return(&model.User{ + Id: mmUserID, + Username: "alice", + Nickname: "Alice", + Email: "alice@example.com", + }, nil) + suite.api.On("GetProfileImage", mmUserID).Return([]byte("fake-image-data"), nil) + + bridgeA := suite.bridgeFor(multiServerAID, suite.containerA.Client) + bridgeB := suite.bridgeFor(multiServerBID, suite.containerB.Client) + + ghostA, err := bridgeA.CreateOrGetGhostUser(mmUserID) + require.NoError(t, err) + ghostB, err := bridgeB.CreateOrGetGhostUser(mmUserID) + require.NoError(t, err) + + // Distinct ghost users, each on its own homeserver. + assert.True(t, strings.HasPrefix(ghostA, "@_mattermost_"), "ghost A %q", ghostA) + assert.True(t, strings.HasSuffix(ghostA, ":"+suite.containerA.ServerDomain), "ghost A %q on server A", ghostA) + assert.True(t, strings.HasSuffix(ghostB, ":"+suite.containerB.ServerDomain), "ghost B %q on server B", ghostB) + assert.NotEqual(t, ghostA, ghostB) + + // The ghost cache is namespaced per server. + cachedA, err := suite.plugin.kvstore.Get(kvstore.BuildGhostUserKey(multiServerAID, mmUserID)) + require.NoError(t, err) + cachedB, err := suite.plugin.kvstore.Get(kvstore.BuildGhostUserKey(multiServerBID, mmUserID)) + require.NoError(t, err) + assert.Equal(t, ghostA, string(cachedA)) + assert.Equal(t, ghostB, string(cachedB)) + + // Re-requesting returns the cached ghost per server (no duplicate creation). + ghostA2, err := bridgeA.CreateOrGetGhostUser(mmUserID) + require.NoError(t, err) + assert.Equal(t, ghostA, ghostA2) +} + +func TestMultiServerIntegrationSuite(t *testing.T) { + suite.Run(t, new(MultiServerIntegrationTestSuite)) +} diff --git a/server/plugin.go b/server/plugin.go index a4f3608..f51269b 100644 --- a/server/plugin.go +++ b/server/plugin.go @@ -37,8 +37,17 @@ type Plugin struct { // commandClient is the client used to register and execute slash commands. commandClient command.Command - // matrixClient is the client used to communicate with Matrix servers. - matrixClient *matrix.Client + // matrixClients holds one Matrix client per configured server, keyed by + // serverID. It currently contains exactly one entry. Guarded by + // matrixClientsLock together with serverID. + matrixClients map[string]*matrix.Client + + // serverID is the cached serverID of the single configured Matrix server, + // populated by reconcileServerConfig. + serverID string + + // matrixClientsLock synchronizes access to matrixClients and serverID. + matrixClientsLock sync.RWMutex // postTracker tracks post creation timestamps to detect redundant edits postTracker *PostTracker @@ -97,7 +106,9 @@ func (p *Plugin) OnActivate() error { p.maxProfileImageSize = DefaultMaxProfileImageSize p.maxFileSize = DefaultMaxFileSize - p.initMatrixClient() + if err := p.initMatrixClient(); err != nil { + return errors.Wrap(err, "failed to initialize Matrix client") + } // Run KV store migrations before initializing bridges if err := p.runKVStoreMigrations(); err != nil { @@ -109,6 +120,14 @@ func (p *Plugin) OnActivate() error { p.logger.LogWarn("Failed to register for shared channels", "error", err) } + // registerForSharedChannels assigns p.remoteID, but the earlier + // initMatrixClient built the clients (and reconciled the registry) before it + // was known. Reinitialize so both the registry's RemoteID and every Matrix + // client carry the assigned remote ID instead of the initial empty value. + if err := p.initMatrixClient(); err != nil { + p.logger.LogWarn("Failed to reinitialize Matrix clients with remote ID", "error", err) + } + // Initialize bridge components after getting remote ID p.initBridges() @@ -148,18 +167,51 @@ func (p *Plugin) ExecuteCommand(_ *plugin.Context, args *model.CommandArgs) (*mo return response, nil } -func (p *Plugin) initMatrixClient() { +func (p *Plugin) initMatrixClient() error { + // OnConfigurationChange can run before OnActivate initializes the KV store. + // The registry lives in the KV store, so defer client setup until it exists; + // OnActivate calls initMatrixClient again once the store is ready. + if p.kvstore == nil { + return nil + } + + // Reconcile the flat plugin.json config into the managed server registry + // first; this mints/keeps the stable serverID that keys the client map. + servers, err := p.reconcileServerConfig() + if err != nil { + // Surface the failure to the caller rather than leaving the client map + // silently stale/empty; a failed reconcile must not look like success. + return errors.Wrap(err, "failed to reconcile server configuration") + } + config := p.getConfiguration() rateLimitMode := matrix.ParseRateLimitingMode(config.RateLimitingMode) rateLimitConfig := matrix.GetRateLimitConfigByMode(rateLimitMode) - p.matrixClient = matrix.NewClientWithRateLimit( - config.MatrixServerURL, - config.MatrixASToken, - p.remoteID, - config.MatrixServerName, - p.API, - rateLimitConfig, - ) + + clients := make(map[string]*matrix.Client, len(servers)) + for _, server := range servers { + clients[server.ServerID] = matrix.NewClientWithRateLimit( + server.ServerURL, + server.ASToken, + p.remoteID, + server.ServerName, + p.API, + rateLimitConfig, + ) + } + + p.matrixClientsLock.Lock() + p.matrixClients = clients + p.matrixClientsLock.Unlock() + return nil +} + +// getMatrixClient returns the Matrix client for the given serverID, or nil if +// none is registered. +func (p *Plugin) getMatrixClient(serverID string) *matrix.Client { + p.matrixClientsLock.RLock() + defer p.matrixClientsLock.RUnlock() + return p.matrixClients[serverID] } func (p *Plugin) initBridges() { @@ -168,7 +220,8 @@ func (p *Plugin) initBridges() { Logger: p.logger, API: p.API, KVStore: p.kvstore, - MatrixClient: p.matrixClient, + MatrixClient: p.GetMatrixClient(), + ServerID: p.getSingleServerID(), RemoteID: p.remoteID, MaxProfileImageSize: p.maxProfileImageSize, MaxFileSize: p.maxFileSize, @@ -220,9 +273,14 @@ func (p *Plugin) registerForSharedChannels() error { // PluginAccessor interface implementation for command handlers -// GetMatrixClient returns the Matrix client instance +// GetMatrixClient returns the Matrix client for the single configured server. func (p *Plugin) GetMatrixClient() *matrix.Client { - return p.matrixClient + return p.getMatrixClient(p.getSingleServerID()) +} + +// GetServerID returns the serverID of the single configured Matrix server. +func (p *Plugin) GetServerID() string { + return p.getSingleServerID() } // GetKVStore returns the KV store instance @@ -289,7 +347,8 @@ func (p *Plugin) UserHasJoinedChannel(_ *plugin.Context, channelMember *model.Ch return } - if p.matrixClient == nil { + matrixClient := p.GetMatrixClient() + if matrixClient == nil { p.logger.LogError("Matrix client not initialized") return } @@ -351,14 +410,14 @@ func (p *Plugin) UserHasJoinedChannel(_ *plugin.Context, channelMember *model.Ch } // Resolve room alias to room ID if needed - resolvedRoomID, err := p.matrixClient.ResolveRoomAlias(matrixRoomID) + resolvedRoomID, err := matrixClient.ResolveRoomAlias(matrixRoomID) if err != nil { p.logger.LogError("Failed to resolve Matrix room identifier", "error", err, "room_identifier", matrixRoomID) return } // Try to join the ghost user to the Matrix room (handles both public and private rooms) - if err := p.matrixClient.InviteAndJoinGhostUser(resolvedRoomID, ghostUserID); err != nil { + if err := matrixClient.InviteAndJoinGhostUser(resolvedRoomID, ghostUserID); err != nil { p.logger.LogError("Failed to join ghost user to Matrix room", "error", err, "ghost_user_id", ghostUserID, "room_id", resolvedRoomID, "mattermost_user_id", user.Id) } else { p.logger.LogInfo("Successfully joined ghost user to Matrix room", "ghost_user_id", ghostUserID, "room_id", resolvedRoomID, "mattermost_user_id", user.Id, "username", user.Username) diff --git a/server/plugin_integration_test.go b/server/plugin_integration_test.go index cbf619e..f141a1a 100644 --- a/server/plugin_integration_test.go +++ b/server/plugin_integration_test.go @@ -11,6 +11,7 @@ import ( "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" + "github.com/mattermost/mattermost-plugin-matrix-bridge/server/store/kvstore" matrixtest "github.com/mattermost/mattermost-plugin-matrix-bridge/testcontainers/matrix" ) @@ -58,8 +59,7 @@ func (suite *PluginIntegrationTestSuite) SetupTest() { // Reuse the container's Matrix client to share rate limiting state // This prevents rate limit conflicts between container setup and plugin operations - suite.plugin.matrixClient = suite.matrixContainer.Client - + setTestMatrixClient(suite.plugin, suite.matrixContainer.Client) // Set configuration config := &configuration{ MatrixServerURL: suite.matrixContainer.ServerURL, @@ -88,7 +88,7 @@ func (suite *PluginIntegrationTestSuite) TestPluginMatrixOperations() { func (suite *PluginIntegrationTestSuite) testInviteRemoteUserToMatrixRoom() { // Create a test room roomIdentifier := suite.matrixContainer.CreateRoom(suite.T(), "Remote User Test Room") - roomID, err := suite.plugin.matrixClient.ResolveRoomAlias(roomIdentifier) + roomID, err := suite.plugin.GetMatrixClient().ResolveRoomAlias(roomIdentifier) require.NoError(suite.T(), err, "Should resolve room identifier") // Create test channel and set up mapping @@ -115,12 +115,12 @@ func (suite *PluginIntegrationTestSuite) testInviteRemoteUserToMatrixRoom() { suite.api.On("GetUser", mattermostUserID).Return(remoteUser, nil) // Set up KV store mapping from Mattermost user to Matrix user - userMapKey := "matrix_user_" + testUser.UserID + userMapKey := kvstore.BuildMatrixUserKey(testServerID, testUser.UserID) err = suite.plugin.kvstore.Set(userMapKey, []byte(mattermostUserID)) require.NoError(suite.T(), err, "Should set up user mapping") // Set up reverse mapping - reverseMapKey := "mattermost_user_" + mattermostUserID + reverseMapKey := kvstore.BuildMattermostUserKey(testServerID, mattermostUserID) err = suite.plugin.kvstore.Set(reverseMapKey, []byte(testUser.UserID)) require.NoError(suite.T(), err, "Should set up reverse user mapping") @@ -175,11 +175,11 @@ func (suite *PluginIntegrationTestSuite) testInviteRemoteUserToMatrixRoom() { suite.api.On("GetUser", mattermostUserID).Return(remoteUser, nil) // Set up user mapping but don't create channel mapping - userMapKey := "matrix_user_" + testUser.UserID + userMapKey := kvstore.BuildMatrixUserKey(testServerID, testUser.UserID) err = suite.plugin.kvstore.Set(userMapKey, []byte(mattermostUserID)) require.NoError(suite.T(), err, "Should set up user mapping") - reverseMapKey := "mattermost_user_" + mattermostUserID + reverseMapKey := kvstore.BuildMattermostUserKey(testServerID, mattermostUserID) err = suite.plugin.kvstore.Set(reverseMapKey, []byte(testUser.UserID)) require.NoError(suite.T(), err, "Should set up reverse user mapping") @@ -197,7 +197,7 @@ func (suite *PluginIntegrationTestSuite) testSyncChannelMembersToMatrixRoom() { // Create test room roomIdentifier := suite.matrixContainer.CreateRoom(suite.T(), "Member Sync Test Room") - roomID, err := suite.plugin.matrixClient.ResolveRoomAlias(roomIdentifier) + roomID, err := suite.plugin.GetMatrixClient().ResolveRoomAlias(roomIdentifier) require.NoError(suite.T(), err, "Should resolve room identifier") // Create test channel @@ -270,19 +270,19 @@ func (suite *PluginIntegrationTestSuite) testSyncChannelMembersToMatrixRoom() { suite.api.On("GetProfileImage", localUser2ID).Return([]byte("fake-image-data-2"), nil) // Set up user mappings for remote users - userMapKey1 := "matrix_user_" + matrixUser1.UserID + userMapKey1 := kvstore.BuildMatrixUserKey(testServerID, matrixUser1.UserID) err = suite.plugin.kvstore.Set(userMapKey1, []byte(remoteUser1ID)) require.NoError(suite.T(), err, "Should set up remote user 1 mapping") - reverseMapKey1 := "mattermost_user_" + remoteUser1ID + reverseMapKey1 := kvstore.BuildMattermostUserKey(testServerID, remoteUser1ID) err = suite.plugin.kvstore.Set(reverseMapKey1, []byte(matrixUser1.UserID)) require.NoError(suite.T(), err, "Should set up reverse mapping for remote user 1") - userMapKey2 := "matrix_user_" + matrixUser2.UserID + userMapKey2 := kvstore.BuildMatrixUserKey(testServerID, matrixUser2.UserID) err = suite.plugin.kvstore.Set(userMapKey2, []byte(remoteUser2ID)) require.NoError(suite.T(), err, "Should set up remote user 2 mapping") - reverseMapKey2 := "mattermost_user_" + remoteUser2ID + reverseMapKey2 := kvstore.BuildMattermostUserKey(testServerID, remoteUser2ID) err = suite.plugin.kvstore.Set(reverseMapKey2, []byte(matrixUser2.UserID)) require.NoError(suite.T(), err, "Should set up reverse mapping for remote user 2") @@ -304,7 +304,7 @@ func (suite *PluginIntegrationTestSuite) testSyncChannelMembersToMatrixRoom() { // Handle remote user - invite to Matrix room originalMatrixUserID, err := suite.plugin.mattermostToMatrixBridge.GetMatrixUserIDFromMattermostUser(user.Id) if err == nil { - err = suite.plugin.matrixClient.InviteUserToRoom(roomID, originalMatrixUserID) + err = suite.plugin.GetMatrixClient().InviteUserToRoom(roomID, originalMatrixUserID) if err == nil { remoteUserCount++ suite.T().Logf("Successfully invited remote user %s (%s) to room", user.Username, originalMatrixUserID) @@ -314,7 +314,7 @@ func (suite *PluginIntegrationTestSuite) testSyncChannelMembersToMatrixRoom() { // Handle local user - create ghost user and join to room ghostUserID, err := suite.plugin.mattermostToMatrixBridge.CreateOrGetGhostUser(user.Id) if err == nil { - err = suite.plugin.matrixClient.InviteAndJoinGhostUser(roomID, ghostUserID) + err = suite.plugin.GetMatrixClient().InviteAndJoinGhostUser(roomID, ghostUserID) if err == nil { localUserCount++ suite.T().Logf("Successfully joined ghost user %s for local user %s to room", ghostUserID, user.Username) @@ -371,7 +371,7 @@ func (suite *PluginIntegrationTestSuite) testSyncChannelMembersToMatrixRoom() { // Create room for empty channel emptyRoomIdentifier := suite.matrixContainer.CreateRoom(suite.T(), "Empty Channel Test Room") - emptyRoomID, err := suite.plugin.matrixClient.ResolveRoomAlias(emptyRoomIdentifier) + emptyRoomID, err := suite.plugin.GetMatrixClient().ResolveRoomAlias(emptyRoomIdentifier) require.NoError(suite.T(), err, "Should resolve empty room identifier") err = suite.plugin.mattermostToMatrixBridge.setChannelRoomMapping(emptyChannelID, emptyRoomID) diff --git a/server/servers.go b/server/servers.go new file mode 100644 index 0000000..b7a43d0 --- /dev/null +++ b/server/servers.go @@ -0,0 +1,165 @@ +package main + +import ( + "crypto/sha256" + "encoding/base32" + "encoding/json" + "strings" + + "github.com/pkg/errors" + + "github.com/mattermost/mattermost-plugin-matrix-bridge/server/matrix" + "github.com/mattermost/mattermost-plugin-matrix-bridge/server/store/kvstore" +) + +// getServers reads the managed Matrix server registry from the KV store. A +// missing or empty registry yields a nil slice and no error. +func (p *Plugin) getServers() ([]kvstore.ServerConfig, error) { + if p.kvstore == nil { + return nil, nil + } + + data, err := p.kvstore.Get(kvstore.KeyServersConfig) + if err != nil { + // A missing key returns (nil, nil) from the plugin KV API, so a non-nil + // error here means a real backend failure. Surface it rather than + // treating it as "no servers registered", so callers never persist to or + // act on a half-read registry. + return nil, errors.Wrap(err, "failed to read servers_config") + } + + servers, err := kvstore.ParseServersConfig(data) + if err != nil { + return nil, errors.Wrap(err, "failed to unmarshal servers_config") + } + return servers, nil +} + +// getSingleServerID returns the serverID of the single configured Matrix server. +// It prefers the value cached during reconcileServerConfig and falls back to +// reading the registry from the KV store. Returns "" if no server is registered. +func (p *Plugin) getSingleServerID() string { + p.matrixClientsLock.RLock() + cached := p.serverID + p.matrixClientsLock.RUnlock() + if cached != "" { + return cached + } + + servers, err := p.getServers() + if err != nil || len(servers) == 0 { + return "" + } + return servers[0].ServerID +} + +// serverIDNamespace returns the "_" prefix used to namespace per-server +// KV keys, or "" when no server is registered yet. Migrations use it to detect +// keys already migrated to the v3 layout. +func (p *Plugin) serverIDNamespace() string { + id := p.getSingleServerID() + if id == "" { + return "" + } + return id + "_" +} + +// reconcileServerConfig derives the managed server registry from the flat +// plugin.json configuration and persists it under KeyServersConfig. The serverID +// is derived deterministically from the homeserver hostname (see deriveServerID), +// so it is stable across restarts and config edits as long as the hostname is +// unchanged. The single serverID is cached for fast lookups. It returns the +// resulting registry. +// +// This is the single authority for establishing the serverID; it needs no Matrix +// client and is idempotent, so it can run safely before migrations and on every +// configuration change. +func (p *Plugin) reconcileServerConfig() ([]kvstore.ServerConfig, error) { + config := p.getConfiguration() + + existing, err := p.getServers() + if err != nil { + return nil, err + } + + // No server URL configured yet (e.g. the plugin is enabled with sync off). + // There is nothing to register: leave any existing registry untouched and + // report it as-is rather than writing a useless entry or failing activation. + // A serverID cannot be derived without a URL, and none is needed yet. + if config.MatrixServerURL == "" { + return existing, nil + } + + // The serverID is derived deterministically from the homeserver hostname, so + // a server re-created with the same URL re-adopts its namespaced KV records + // instead of orphaning them. We always derive rather than reuse the stored + // ID: that determinism is what makes the recovery possible. + serverID, err := deriveServerID(config.MatrixServerURL) + if err != nil { + return nil, err + } + + persistedRemoteID := "" + if len(existing) > 0 { + persistedRemoteID = existing[0].RemoteID + if existing[0].ServerID != serverID { + p.logger.LogWarn("Matrix homeserver hostname changed; KV records under the previous serverID are now orphaned and can be recovered by reverting the server URL", + "previous_server_id", existing[0].ServerID, "new_server_id", serverID) + } + } + + // reconcileServerConfig runs during initMatrixClient before + // registerForSharedChannels has assigned p.remoteID. Preserve the already + // persisted RemoteID in that window so an early reconcile (or a failed + // registration) never erases a valid remote identity. + remoteID := p.remoteID + if remoteID == "" { + remoteID = persistedRemoteID + } + + entry := kvstore.ServerConfig{ + ServerID: serverID, + ServerURL: config.MatrixServerURL, + ServerName: config.MatrixServerName, + ASToken: config.MatrixASToken, + HSToken: config.MatrixHSToken, + UsernamePrefix: config.GetMatrixUsernamePrefix(), + Enabled: config.EnableSync, + RemoteID: remoteID, + } + servers := []kvstore.ServerConfig{entry} + + data, err := json.Marshal(servers) + if err != nil { + return nil, errors.Wrap(err, "failed to marshal servers_config") + } + if err := p.kvstore.Set(kvstore.KeyServersConfig, data); err != nil { + return nil, errors.Wrap(err, "failed to persist servers_config") + } + + p.matrixClientsLock.Lock() + p.serverID = serverID + p.matrixClientsLock.Unlock() + + return servers, nil +} + +// serverIDEncoding matches model.NewId()'s base32 alphabet so a derived serverID +// is indistinguishable in shape from a framework-minted ID. +var serverIDEncoding = base32.NewEncoding("ybndrfg8ejkmcpqxot1uwisza345h769").WithPadding(base32.NoPadding) + +// deriveServerID produces the stable, deterministic serverID for a homeserver +// from its base URL. It hashes the normalized hostname (case-folded; scheme, +// port and path stripped) so the same server always yields the same ID. That +// determinism is what lets orphaned KV records be re-adopted when a registry +// entry is lost and later re-created with the same URL. The output is a +// 26-character string in Mattermost's base32 ID alphabet, a drop-in replacement +// for model.NewId() as a KV namespace key. +func deriveServerID(serverURL string) (string, error) { + host, err := matrix.ExtractServerDomain(serverURL) + if err != nil { + return "", errors.Wrap(err, "cannot derive serverID from server URL") + } + sum := sha256.Sum256([]byte(strings.ToLower(host))) + return serverIDEncoding.EncodeToString(sum[:16])[:26], nil +} diff --git a/server/servers_test.go b/server/servers_test.go new file mode 100644 index 0000000..d4a45fb --- /dev/null +++ b/server/servers_test.go @@ -0,0 +1,425 @@ +package main + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/mattermost/mattermost-plugin-matrix-bridge/server/store/kvstore" +) + +func TestReconcileServerConfig(t *testing.T) { + t.Run("MintsAndDerivesFieldsFromFlatConfig", func(t *testing.T) { + plugin := setupPluginForTest() + plugin.kvstore = NewMemoryKVStore() + plugin.logger = &testLogger{t: t} + plugin.remoteID = "remote-xyz" + plugin.configuration = &configuration{ + MatrixServerURL: "https://matrix.example.com", + MatrixServerName: "example.com", + MatrixASToken: "as-token", + MatrixHSToken: "hs-token", + MatrixUsernamePrefix: "mx", + EnableSync: true, + } + + servers, err := plugin.reconcileServerConfig() + require.NoError(t, err) + require.Len(t, servers, 1) + + s := servers[0] + expectedID, err := deriveServerID("https://matrix.example.com") + require.NoError(t, err) + assert.Equal(t, expectedID, s.ServerID, "serverID is derived from the URL hostname") + assert.Equal(t, "https://matrix.example.com", s.ServerURL) + assert.Equal(t, "example.com", s.ServerName) + assert.Equal(t, "as-token", s.ASToken) + assert.Equal(t, "hs-token", s.HSToken) + assert.Equal(t, "mx", s.UsernamePrefix) + assert.True(t, s.Enabled) + assert.Equal(t, "remote-xyz", s.RemoteID) + + // The entry is persisted and reloads to the same serverID. + reloaded, err := plugin.getServers() + require.NoError(t, err) + require.Len(t, reloaded, 1) + assert.Equal(t, s.ServerID, reloaded[0].ServerID) + }) + + t.Run("ServerIDStableWhenHostnameUnchanged", func(t *testing.T) { + plugin := setupPluginForTest() + plugin.kvstore = NewMemoryKVStore() + plugin.logger = &testLogger{t: t} + plugin.configuration = &configuration{MatrixServerURL: "https://a.example.com"} + + first, err := plugin.reconcileServerConfig() + require.NoError(t, err) + require.Len(t, first, 1) + originalID := first[0].ServerID + require.NotEmpty(t, originalID) + + // A config edit that touches other fields but keeps the same hostname + // (here also normalizing scheme/port/trailing-slash) must keep the same + // serverID, even from a fresh process (cache cleared). + plugin.configuration = &configuration{MatrixServerURL: "http://a.example.com:8008/", MatrixASToken: "new"} + plugin.serverID = "" + + second, err := plugin.reconcileServerConfig() + require.NoError(t, err) + require.Len(t, second, 1) + assert.Equal(t, originalID, second[0].ServerID, "serverID is stable while the hostname is unchanged") + assert.Equal(t, "http://a.example.com:8008/", second[0].ServerURL, "other fields update from flat config") + }) + + t.Run("ServerIDChangesWhenHostnameChanges", func(t *testing.T) { + plugin := setupPluginForTest() + plugin.kvstore = NewMemoryKVStore() + plugin.logger = &testLogger{t: t} + plugin.configuration = &configuration{MatrixServerURL: "https://a.example.com"} + + first, err := plugin.reconcileServerConfig() + require.NoError(t, err) + originalID := first[0].ServerID + + // Repointing to a different homeserver hostname derives a new serverID + // (records under the old ID are orphaned; a warning is logged). + plugin.configuration = &configuration{MatrixServerURL: "https://b.example.com"} + plugin.serverID = "" + + second, err := plugin.reconcileServerConfig() + require.NoError(t, err) + require.Len(t, second, 1) + expectedID, err := deriveServerID("https://b.example.com") + require.NoError(t, err) + assert.Equal(t, expectedID, second[0].ServerID) + assert.NotEqual(t, originalID, second[0].ServerID, "a hostname change re-derives the serverID") + }) + + t.Run("NoEntryWhenServerURLEmpty", func(t *testing.T) { + plugin := setupPluginForTest() + plugin.kvstore = NewMemoryKVStore() + plugin.logger = &testLogger{t: t} + plugin.configuration = &configuration{MatrixServerURL: ""} + + // No URL configured (e.g. sync disabled): reconcile is a no-op that must + // not fail activation and must not write a useless entry. + servers, err := plugin.reconcileServerConfig() + require.NoError(t, err) + assert.Empty(t, servers, "no server URL means no registry entry") + + reloaded, err := plugin.getServers() + require.NoError(t, err) + assert.Empty(t, reloaded) + }) + + t.Run("ErrorsOnUnparseableServerURL", func(t *testing.T) { + plugin := setupPluginForTest() + plugin.kvstore = NewMemoryKVStore() + plugin.logger = &testLogger{t: t} + plugin.configuration = &configuration{MatrixServerURL: "http://"} + + _, err := plugin.reconcileServerConfig() + require.Error(t, err, "a non-empty but unusable URL must fail loudly, not derive an empty serverID") + }) + + t.Run("ReReconcileUpdatesDerivedFields", func(t *testing.T) { + plugin := setupPluginForTest() + plugin.kvstore = NewMemoryKVStore() + plugin.logger = &testLogger{t: t} + plugin.remoteID = "remote-1" + plugin.configuration = &configuration{MatrixServerURL: "https://a.example.com", EnableSync: true} + + first, err := plugin.reconcileServerConfig() + require.NoError(t, err) + require.Len(t, first, 1) + assert.Equal(t, "remote-1", first[0].RemoteID) + assert.True(t, first[0].Enabled) + originalID := first[0].ServerID + + // A later reconcile (e.g. after shared-channels registration and a config + // edit that disables sync) refreshes the derived fields but keeps serverID. + plugin.remoteID = "remote-2" + plugin.configuration = &configuration{MatrixServerURL: "https://a.example.com", EnableSync: false} + + second, err := plugin.reconcileServerConfig() + require.NoError(t, err) + require.Len(t, second, 1) + assert.Equal(t, originalID, second[0].ServerID) + assert.Equal(t, "remote-2", second[0].RemoteID) + assert.False(t, second[0].Enabled) + }) + + t.Run("DoesNotMintOnReadError", func(t *testing.T) { + base := NewMemoryKVStore() + plugin := setupPluginForTest() + plugin.logger = &testLogger{t: t} + plugin.configuration = &configuration{MatrixServerURL: "https://a.example.com"} + plugin.kvstore = base + + // An existing registry with a stable serverID. + seedTestServerConfig(plugin) + + // Reads of servers_config now fail (transient backend error). + plugin.kvstore = &failOnGetKVStore{KVStore: base, failKeySubstr: kvstore.KeyServersConfig} + plugin.serverID = "" // force a registry read rather than using the cache + + _, err := plugin.getServers() + require.Error(t, err, "a real read failure must surface, not look like an empty registry") + + _, err = plugin.reconcileServerConfig() + require.Error(t, err, "reconcile must fail rather than mint a fresh serverID on a read error") + + // The persisted serverID is unchanged — no re-mint, no orphaned records. + plugin.kvstore = base + servers, err := plugin.getServers() + require.NoError(t, err) + require.Len(t, servers, 1) + assert.Equal(t, testServerID, servers[0].ServerID) + }) + + t.Run("GetServersNilKVStore", func(t *testing.T) { + plugin := setupPluginForTest() + servers, err := plugin.getServers() + require.NoError(t, err) + assert.Nil(t, servers) + }) + + t.Run("GetServersMalformedJSONReturnsError", func(t *testing.T) { + plugin := setupPluginForTest() + plugin.kvstore = NewMemoryKVStore() + require.NoError(t, plugin.kvstore.Set(kvstore.KeyServersConfig, []byte("not json"))) + + _, err := plugin.getServers() + assert.Error(t, err) + }) +} + +// TestDeriveServerID verifies the deterministic serverID derivation: same +// hostname (regardless of scheme/port/path/case) yields the same 26-char base32 +// ID, distinct hostnames yield distinct IDs, and an unusable URL errors. +func TestDeriveServerID(t *testing.T) { + const base32Alphabet = "ybndrfg8ejkmcpqxot1uwisza345h769" + + t.Run("DeterministicAndFormatted", func(t *testing.T) { + id, err := deriveServerID("https://matrix.example.com") + require.NoError(t, err) + assert.Len(t, id, 26, "matches model.NewId() length") + for _, r := range id { + assert.Contains(t, base32Alphabet, string(r), "only Mattermost base32 alphabet chars") + } + + again, err := deriveServerID("https://matrix.example.com") + require.NoError(t, err) + assert.Equal(t, id, again, "same input is deterministic") + }) + + t.Run("NormalizationEquivalence", func(t *testing.T) { + want, err := deriveServerID("https://matrix.example.com") + require.NoError(t, err) + + for _, u := range []string{ + "http://matrix.example.com", + "https://matrix.example.com:8008", + "https://matrix.example.com/", + "https://matrix.example.com/_matrix", + "HTTPS://MATRIX.EXAMPLE.COM", + } { + got, err := deriveServerID(u) + require.NoError(t, err, u) + assert.Equal(t, want, got, "scheme/port/path/case must not change the ID: %s", u) + } + }) + + t.Run("DistinctHostnamesDiffer", func(t *testing.T) { + a, err := deriveServerID("https://a.example.com") + require.NoError(t, err) + b, err := deriveServerID("https://b.example.com") + require.NoError(t, err) + assert.NotEqual(t, a, b) + }) + + t.Run("ErrorsOnUnusableURL", func(t *testing.T) { + for _, u := range []string{"", "not a url"} { + _, err := deriveServerID(u) + assert.Error(t, err, "expected error for %q", u) + } + }) +} + +// TestMatrixUsernamePrefixResolvesPerServer verifies the username prefix is +// resolved from the per-server registry entry (the source of truth), and that +// the flat global config is not consulted at resolution time. +func TestMatrixUsernamePrefixResolvesPerServer(t *testing.T) { + newBridge := func(t *testing.T) *Plugin { + plugin := setupPluginForTest() + plugin.kvstore = NewMemoryKVStore() + plugin.logger = &testLogger{t: t} + plugin.maxProfileImageSize = DefaultMaxProfileImageSize + plugin.maxFileSize = DefaultMaxFileSize + plugin.postTracker = NewPostTracker(DefaultPostTrackerMaxEntries) + plugin.pendingFiles = NewPendingFileTracker() + // A custom global prefix that must NOT leak into resolution; only the + // registry entry (or the static default) may be returned. + plugin.configuration = &configuration{MatrixUsernamePrefix: "globalprefix"} + setTestMatrixClient(plugin, createMatrixClientWithTestLogger(t, "", "", "")) + plugin.initBridges() + return plugin + } + + t.Run("UsesRegistryEntryPrefix", func(t *testing.T) { + plugin := newBridge(t) + servers := []kvstore.ServerConfig{{ServerID: testServerID, UsernamePrefix: "serverprefix"}} + data, err := json.Marshal(servers) + require.NoError(t, err) + require.NoError(t, plugin.kvstore.Set(kvstore.KeyServersConfig, data)) + + assert.Equal(t, "serverprefix", plugin.mattermostToMatrixBridge.matrixUsernamePrefix()) + }) + + t.Run("DefaultsWhenNoRegistryEntry", func(t *testing.T) { + plugin := newBridge(t) + // No servers_config seeded: resolves to the static default, NOT the + // configured global prefix. + assert.Equal(t, DefaultMatrixUsernamePrefix, plugin.mattermostToMatrixBridge.matrixUsernamePrefix()) + }) + + t.Run("DefaultsWhenEntryPrefixEmpty", func(t *testing.T) { + plugin := newBridge(t) + servers := []kvstore.ServerConfig{{ServerID: testServerID, UsernamePrefix: ""}} + data, err := json.Marshal(servers) + require.NoError(t, err) + require.NoError(t, plugin.kvstore.Set(kvstore.KeyServersConfig, data)) + + assert.Equal(t, DefaultMatrixUsernamePrefix, plugin.mattermostToMatrixBridge.matrixUsernamePrefix()) + }) + + t.Run("IgnoresOtherServersEntry", func(t *testing.T) { + plugin := newBridge(t) + // The registry only has an entry for a DIFFERENT server; this bridge's + // server must not pick up another server's prefix. + servers := []kvstore.ServerConfig{{ServerID: "some-other-server", UsernamePrefix: "otherprefix"}} + data, err := json.Marshal(servers) + require.NoError(t, err) + require.NoError(t, plugin.kvstore.Set(kvstore.KeyServersConfig, data)) + + assert.Equal(t, DefaultMatrixUsernamePrefix, plugin.mattermostToMatrixBridge.matrixUsernamePrefix()) + }) +} + +// TestGetMatrixRoomIDServerIsolation exercises the per-server filtering in +// GetMatrixRoomID / RoomIDForServer, which existing tests never hit because they +// always read and write with the same serverID. +func TestGetMatrixRoomIDServerIsolation(t *testing.T) { + plugin := setupPluginForTest() + plugin.kvstore = NewMemoryKVStore() + plugin.logger = &testLogger{t: t} + plugin.maxProfileImageSize = DefaultMaxProfileImageSize + plugin.maxFileSize = DefaultMaxFileSize + plugin.postTracker = NewPostTracker(DefaultPostTrackerMaxEntries) + plugin.pendingFiles = NewPendingFileTracker() + setTestMatrixClient(plugin, createMatrixClientWithTestLogger(t, "", "", "")) + plugin.initBridges() + bridge := plugin.mattermostToMatrixBridge + + channelID := "channelABC" + key := kvstore.BuildChannelMappingKey(channelID) + + t.Run("MatchingServerIDReturnsRoom", func(t *testing.T) { + v, err := kvstore.BuildSingleChannelMapping(testServerID, "!room:hs") + require.NoError(t, err) + require.NoError(t, plugin.kvstore.Set(key, v)) + + got, err := bridge.GetMatrixRoomID(channelID) + require.NoError(t, err) + assert.Equal(t, "!room:hs", got) + }) + + t.Run("DifferentServerIDReturnsEmpty", func(t *testing.T) { + v, err := kvstore.BuildSingleChannelMapping("some-other-server", "!other:hs") + require.NoError(t, err) + require.NoError(t, plugin.kvstore.Set(key, v)) + + got, err := bridge.GetMatrixRoomID(channelID) + require.NoError(t, err) + assert.Equal(t, "", got, "a mapping for a different server must not resolve") + }) + + t.Run("CorruptValueReturnsError", func(t *testing.T) { + // An unparseable value is surfaced as an error rather than masked as an + // unmapped channel (which would silently drop/mis-route messages). + require.NoError(t, plugin.kvstore.Set(key, []byte("!not-json:hs"))) + + _, err := bridge.GetMatrixRoomID(channelID) + require.Error(t, err) + }) +} + +// TestSetChannelRoomMappingPreservesOtherServers verifies the write path upserts +// only this server's entry, leaving mappings for other servers intact. +func TestSetChannelRoomMappingPreservesOtherServers(t *testing.T) { + plugin := setupPluginForTest() + plugin.kvstore = NewMemoryKVStore() + plugin.logger = &testLogger{t: t} + + channelID := "channelMulti" + key := kvstore.BuildChannelMappingKey(channelID) + + // A pre-existing mapping for a different server. + seed, err := kvstore.MarshalChannelServerMappings([]kvstore.ChannelServerMapping{ + {ServerID: "other-server", RoomID: "!other:hs"}, + }) + require.NoError(t, err) + require.NoError(t, plugin.kvstore.Set(key, seed)) + + // A bridge for testServerID with a stub client so ResolveRoomAlias resolves. + setTestMatrixClient(plugin, createMatrixClientWithTestLogger(t, "", "", "")) + utils := NewBridgeUtils(BridgeUtilsConfig{ + Logger: plugin.logger, + API: plugin.API, + KVStore: plugin.kvstore, + MatrixClient: plugin.GetMatrixClient(), + ServerID: testServerID, + ConfigGetter: plugin, + }) + + require.NoError(t, utils.setChannelRoomMapping(channelID, "!mine:hs")) + + data, err := plugin.kvstore.Get(key) + require.NoError(t, err) + mappings, err := kvstore.ParseChannelServerMappings(data) + require.NoError(t, err) + + // Both servers' entries are present. + assert.Equal(t, "!other:hs", kvstore.RoomIDForServer(mappings, "other-server"), "other server's mapping must be preserved") + assert.Equal(t, "!mine:hs", kvstore.RoomIDForServer(mappings, testServerID), "this server's mapping must be upserted") + assert.Len(t, mappings, 2) +} + +// TestSetChannelRoomMappingRequiresServerID verifies the guard that refuses to +// persist a channel mapping when the serverID is unset, which would otherwise +// write an unroutable mapping and a corrupt reverse key. +func TestSetChannelRoomMappingRequiresServerID(t *testing.T) { + plugin := setupPluginForTest() + plugin.kvstore = NewMemoryKVStore() + plugin.logger = &testLogger{t: t} + + utils := NewBridgeUtils(BridgeUtilsConfig{ + Logger: plugin.logger, + API: plugin.API, + KVStore: plugin.kvstore, + MatrixClient: nil, // guard returns before the client is used + ServerID: "", + RemoteID: "test-remote-id", + ConfigGetter: plugin, + }) + + err := utils.setChannelRoomMapping("channel1", "!room:matrix.org") + require.Error(t, err, "must not persist a channel mapping without a serverID") + + // Nothing was written. + data, err := plugin.kvstore.Get(kvstore.BuildChannelMappingKey("channel1")) + require.NoError(t, err) + assert.Empty(t, data) +} diff --git a/server/store/kvstore/constants.go b/server/store/kvstore/constants.go index a100e21..c664694 100644 --- a/server/store/kvstore/constants.go +++ b/server/store/kvstore/constants.go @@ -6,7 +6,7 @@ package kvstore const ( // CurrentKVStoreVersion is the current version requiring migrations - CurrentKVStoreVersion = 2 + CurrentKVStoreVersion = 3 // KeyPrefixMatrixUser is the prefix for Matrix user ID -> Mattermost user ID mappings KeyPrefixMatrixUser = "matrix_user_" // KeyPrefixMattermostUser is the prefix for Mattermost user ID -> Matrix user ID mappings @@ -30,6 +30,9 @@ const ( // KeyStoreVersion is the key for tracking the current KV store schema version KeyStoreVersion = "kv_store_version" + // KeyServersConfig is the key for the managed Matrix server registry (JSON array of ServerConfig) + KeyServersConfig = "servers_config" + // KeyPrefixLegacyDMMapping was the old prefix for DM mappings (migrated to channel_mapping_) KeyPrefixLegacyDMMapping = "dm_mapping_" // KeyPrefixLegacyMatrixDMMapping was the old prefix for Matrix DM mappings (migrated to room_mapping_) @@ -37,43 +40,52 @@ const ( ) // Helper functions for building KV store keys +// +// Per-server keys are namespaced by a stable serverID derived deterministically +// from each configured Matrix homeserver's hostname, producing keys of the form +// "_". This keeps every homeserver's mappings isolated so +// the plugin can bridge more than one server. The channel_mapping_ key is the +// exception: its key stays server-agnostic and the per-server association lives +// in its value (see ChannelServerMapping). // BuildMatrixUserKey creates a key for Matrix user -> Mattermost user mapping -func BuildMatrixUserKey(matrixUserID string) string { - return KeyPrefixMatrixUser + matrixUserID +func BuildMatrixUserKey(serverID, matrixUserID string) string { + return KeyPrefixMatrixUser + serverID + "_" + matrixUserID } // BuildMattermostUserKey creates a key for Mattermost user -> Matrix user mapping -func BuildMattermostUserKey(mattermostUserID string) string { - return KeyPrefixMattermostUser + mattermostUserID +func BuildMattermostUserKey(serverID, mattermostUserID string) string { + return KeyPrefixMattermostUser + serverID + "_" + mattermostUserID } -// BuildChannelMappingKey creates a key for channel -> room mapping +// BuildChannelMappingKey creates a key for channel -> room mapping. The key is +// intentionally not namespaced by serverID; the server association is carried in +// the value (a []ChannelServerMapping). func BuildChannelMappingKey(channelID string) string { return KeyPrefixChannelMapping + channelID } // BuildRoomMappingKey creates a key for room -> channel mapping -func BuildRoomMappingKey(roomIdentifier string) string { - return KeyPrefixRoomMapping + roomIdentifier +func BuildRoomMappingKey(serverID, roomIdentifier string) string { + return KeyPrefixRoomMapping + serverID + "_" + roomIdentifier } // BuildGhostUserKey creates a key for ghost user cache -func BuildGhostUserKey(mattermostUserID string) string { - return KeyPrefixGhostUser + mattermostUserID +func BuildGhostUserKey(serverID, mattermostUserID string) string { + return KeyPrefixGhostUser + serverID + "_" + mattermostUserID } // BuildGhostRoomKey creates a key for ghost user room membership -func BuildGhostRoomKey(mattermostUserID, roomID string) string { - return KeyPrefixGhostRoom + mattermostUserID + "_" + roomID +func BuildGhostRoomKey(serverID, mattermostUserID, roomID string) string { + return KeyPrefixGhostRoom + serverID + "_" + mattermostUserID + "_" + roomID } // BuildMatrixEventPostKey creates a key for Matrix event -> post mapping -func BuildMatrixEventPostKey(matrixEventID string) string { - return KeyPrefixMatrixEventPost + matrixEventID +func BuildMatrixEventPostKey(serverID, matrixEventID string) string { + return KeyPrefixMatrixEventPost + serverID + "_" + matrixEventID } // BuildMatrixReactionKey creates a key for Matrix reaction storage -func BuildMatrixReactionKey(reactionEventID string) string { - return KeyPrefixMatrixReaction + reactionEventID +func BuildMatrixReactionKey(serverID, reactionEventID string) string { + return KeyPrefixMatrixReaction + serverID + "_" + reactionEventID } diff --git a/server/store/kvstore/schema.go b/server/store/kvstore/schema.go new file mode 100644 index 0000000..732172b --- /dev/null +++ b/server/store/kvstore/schema.go @@ -0,0 +1,120 @@ +package kvstore + +import "encoding/json" + +// This file defines the value schemas for structured KV records. Key builders +// and prefixes live in constants.go. + +// ServerConfig is a single entry in the managed Matrix server registry, +// persisted as a JSON array under the KeyServersConfig key. The registry +// currently holds exactly one entry, derived from the flat plugin.json +// configuration, but the shape supports multiple homeservers. +type ServerConfig struct { + // ServerID is a stable identifier derived deterministically from the + // homeserver hostname (see deriveServerID). It is the join key for every + // per-server KV record and stays constant as long as the hostname is + // unchanged, so a server re-created with the same URL re-adopts its records. + ServerID string `json:"server_id"` + // ServerURL is the Matrix homeserver base URL. + ServerURL string `json:"server_url"` + // ServerName is the Matrix ID domain. May be empty, in which case it is + // resolved via server discovery (.well-known). + ServerName string `json:"server_name"` + // ASToken is the Application Service token. + ASToken string `json:"as_token"` + // HSToken is the Homeserver token. + HSToken string `json:"hs_token"` + // UsernamePrefix is the prefix applied to Matrix-originated usernames. + UsernamePrefix string `json:"username_prefix"` + // Enabled indicates whether this server participates in sync. Populated but + // not yet independently toggle-able via UI. + Enabled bool `json:"enabled"` + // RemoteID is the shared-channels remote identifier, currently the single + // global remoteID returned by RegisterPluginForSharedChannels. + RemoteID string `json:"remote_id"` +} + +// ChannelServerMapping is one element of the value stored under a +// channel_mapping_ key. It associates a Mattermost channel with a +// Matrix room on a specific server. The list currently always has length 1. +type ChannelServerMapping struct { + // ServerID is the serverID of the Matrix server hosting RoomID. + ServerID string `json:"server_id"` + // RoomID is the mapped Matrix room ID (or alias) on that server. + RoomID string `json:"room_id"` +} + +// ParseServersConfig unmarshals a servers_config value (the managed server +// registry). An empty value yields a nil slice and no error; a malformed value +// returns an error so callers never mistake a corrupt registry for "no servers". +func ParseServersConfig(data []byte) ([]ServerConfig, error) { + if len(data) == 0 { + return nil, nil + } + var servers []ServerConfig + if err := json.Unmarshal(data, &servers); err != nil { + return nil, err + } + return servers, nil +} + +// ServerConfigForID returns the registry entry for the given serverID and true, +// or a zero entry and false if none matches. +func ServerConfigForID(servers []ServerConfig, serverID string) (ServerConfig, bool) { + for _, s := range servers { + if s.ServerID == serverID { + return s, true + } + } + return ServerConfig{}, false +} + +// ParseChannelServerMappings unmarshals a channel_mapping_ value. An empty value +// yields a nil slice and no error. A malformed (non-JSON) value returns an error +// so callers can distinguish an unmapped channel from a corrupt record; after the +// v3 migration all stored values are well-formed JSON arrays. +func ParseChannelServerMappings(data []byte) ([]ChannelServerMapping, error) { + if len(data) == 0 { + return nil, nil + } + var mappings []ChannelServerMapping + if err := json.Unmarshal(data, &mappings); err != nil { + return nil, err + } + return mappings, nil +} + +// MarshalChannelServerMappings serializes a channel_mapping_ value. +func MarshalChannelServerMappings(mappings []ChannelServerMapping) ([]byte, error) { + return json.Marshal(mappings) +} + +// BuildSingleChannelMapping serializes a single-entry channel_mapping_ value, the +// only shape currently produced. +func BuildSingleChannelMapping(serverID, roomID string) ([]byte, error) { + return MarshalChannelServerMappings([]ChannelServerMapping{{ServerID: serverID, RoomID: roomID}}) +} + +// UpsertChannelServerMapping sets the RoomID for serverID within mappings, +// replacing an existing entry for that server or appending a new one, and +// returns the result. Entries for other servers are preserved so a channel can +// be mapped to rooms on multiple homeservers. +func UpsertChannelServerMapping(mappings []ChannelServerMapping, serverID, roomID string) []ChannelServerMapping { + for i := range mappings { + if mappings[i].ServerID == serverID { + mappings[i].RoomID = roomID + return mappings + } + } + return append(mappings, ChannelServerMapping{ServerID: serverID, RoomID: roomID}) +} + +// RoomIDForServer returns the RoomID mapped for the given serverID, or "" if none. +func RoomIDForServer(mappings []ChannelServerMapping, serverID string) string { + for _, m := range mappings { + if m.ServerID == serverID { + return m.RoomID + } + } + return "" +} diff --git a/server/store/kvstore/schema_test.go b/server/store/kvstore/schema_test.go new file mode 100644 index 0000000..2a7b568 --- /dev/null +++ b/server/store/kvstore/schema_test.go @@ -0,0 +1,48 @@ +package kvstore + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParseChannelServerMappings(t *testing.T) { + t.Run("EmptyInput", func(t *testing.T) { + mappings, err := ParseChannelServerMappings(nil) + require.NoError(t, err) + assert.Nil(t, mappings) + }) + + t.Run("MalformedInputReturnsError", func(t *testing.T) { + // A bare room ID / alias is the legacy value shape and is not valid JSON. + _, err := ParseChannelServerMappings([]byte("!room:hs")) + assert.Error(t, err) + _, err = ParseChannelServerMappings([]byte("#alias:hs")) + assert.Error(t, err) + }) + + t.Run("RoundTripsSingleMapping", func(t *testing.T) { + data, err := BuildSingleChannelMapping("srv1", "!room:hs") + require.NoError(t, err) + + mappings, err := ParseChannelServerMappings(data) + require.NoError(t, err) + require.Len(t, mappings, 1) + assert.Equal(t, "srv1", mappings[0].ServerID) + assert.Equal(t, "!room:hs", mappings[0].RoomID) + }) +} + +func TestRoomIDForServer(t *testing.T) { + mappings := []ChannelServerMapping{ + {ServerID: "srv1", RoomID: "!one:hs"}, + {ServerID: "srv2", RoomID: "!two:hs"}, + } + + assert.Equal(t, "!one:hs", RoomIDForServer(mappings, "srv1")) + assert.Equal(t, "!two:hs", RoomIDForServer(mappings, "srv2")) + assert.Equal(t, "", RoomIDForServer(mappings, "srv3"), "unknown server yields no room") + assert.Equal(t, "", RoomIDForServer(mappings, ""), "empty server yields no room") + assert.Equal(t, "", RoomIDForServer(nil, "srv1"), "nil mappings yield no room") +} diff --git a/server/sync_to_matrix.go b/server/sync_to_matrix.go index 4b4fb2c..3f3fd4c 100644 --- a/server/sync_to_matrix.go +++ b/server/sync_to_matrix.go @@ -45,7 +45,7 @@ func NewMattermostToMatrixBridge(utils *BridgeUtils, fileTracker FileTracker, po // MattermostToMatrix-specific utility methods func (b *MattermostToMatrixBridge) getGhostUser(userID string) (string, bool) { - ghostUserKey := kvstore.BuildGhostUserKey(userID) + ghostUserKey := kvstore.BuildGhostUserKey(b.serverID, userID) ghostUserIDBytes, err := b.kvstore.Get(ghostUserKey) if err == nil && len(ghostUserIDBytes) > 0 { return string(ghostUserIDBytes), true @@ -91,7 +91,7 @@ func (b *MattermostToMatrixBridge) CreateOrGetGhostUser(userID string) (string, } // Cache the ghost user ID - ghostUserKey := kvstore.BuildGhostUserKey(userID) + ghostUserKey := kvstore.BuildGhostUserKey(b.serverID, userID) err = b.kvstore.Set(ghostUserKey, []byte(ghostUser.UserID)) if err != nil { b.logger.LogWarn("Failed to cache ghost user ID", "error", err, "ghost_user_id", ghostUser.UserID) @@ -108,7 +108,7 @@ func (b *MattermostToMatrixBridge) CreateOrGetGhostUser(userID string) (string, func (b *MattermostToMatrixBridge) ensureGhostUserInRoom(ghostUserID, roomID, userID string) error { // Check if we've already confirmed this ghost user is in this room - roomMembershipKey := kvstore.BuildGhostRoomKey(userID, roomID) + roomMembershipKey := kvstore.BuildGhostRoomKey(b.serverID, userID, roomID) membershipBytes, err := b.kvstore.Get(roomMembershipKey) if err == nil && len(membershipBytes) > 0 && string(membershipBytes) == "joined" { // Already confirmed this user is in the room @@ -1281,7 +1281,7 @@ func (b *MattermostToMatrixBridge) getOrCreateDMRoom(channelID string, userIDs [ // If KV lookup fails, attempts to reconstruct the Matrix user ID from the username func (b *MattermostToMatrixBridge) GetMatrixUserIDFromMattermostUser(mattermostUserID string) (string, error) { // Use Mattermost user ID as key: mattermost_user_ -> matrixUserID - mattermostUserKey := kvstore.BuildMattermostUserKey(mattermostUserID) + mattermostUserKey := kvstore.BuildMattermostUserKey(b.serverID, mattermostUserID) matrixUserIDBytes, err := b.kvstore.Get(mattermostUserKey) if err == nil && len(matrixUserIDBytes) > 0 { return string(matrixUserIDBytes), nil diff --git a/server/sync_to_matrix_integration_test.go b/server/sync_to_matrix_integration_test.go index 071cb3a..9525362 100644 --- a/server/sync_to_matrix_integration_test.go +++ b/server/sync_to_matrix_integration_test.go @@ -66,14 +66,14 @@ func (suite *MatrixSyncTestSuite) SetupTest() { suite.plugin.postTracker = NewPostTracker(DefaultPostTrackerMaxEntries) // Create Matrix client pointing to test container - suite.plugin.matrixClient = createMatrixClientWithTestLogger( + setTestMatrixClient(suite.plugin, createMatrixClientWithTestLogger( suite.T(), suite.matrixContainer.ServerURL, suite.matrixContainer.ASToken, suite.plugin.remoteID, - ) + )) // Set explicit server domain for testing - suite.plugin.matrixClient.SetServerDomain(suite.matrixContainer.ServerDomain) + suite.plugin.GetMatrixClient().SetServerDomain(suite.matrixContainer.ServerDomain) // Set up configuration config := &configuration{ diff --git a/server/sync_to_matrix_test.go b/server/sync_to_matrix_test.go index c28caad..2b749ad 100644 --- a/server/sync_to_matrix_test.go +++ b/server/sync_to_matrix_test.go @@ -19,7 +19,7 @@ func TestCompareTextContent(t *testing.T) { plugin.pendingFiles = NewPendingFileTracker() plugin.client = pluginapi.NewClient(plugin.API, nil) plugin.kvstore = kvstore.NewKVStore(plugin.client) - plugin.matrixClient = createMatrixClientWithTestLogger(t, "", "", "") + setTestMatrixClient(plugin, createMatrixClientWithTestLogger(t, "", "", "")) // Initialize bridges for testing plugin.initBridges() diff --git a/server/sync_to_mattermost.go b/server/sync_to_mattermost.go index 0683784..33cbd96 100644 --- a/server/sync_to_mattermost.go +++ b/server/sync_to_mattermost.go @@ -360,7 +360,7 @@ func (b *MatrixToMattermostBridge) syncMatrixReactionToMattermost(event MatrixEv // Store mapping of Matrix reaction event ID to Mattermost reaction info for deletion purposes // We need to store the info to reconstruct the model.Reaction object for deletion - reactionKey := kvstore.BuildMatrixReactionKey(event.EventID) + reactionKey := kvstore.BuildMatrixReactionKey(b.serverID, event.EventID) reactionInfo := map[string]string{ "post_id": postID, "user_id": mattermostUserID, @@ -447,7 +447,7 @@ func (b *MatrixToMattermostBridge) handleReactionDeletion(redactedReactionEvent } // Look up the stored Mattermost reaction info using the Matrix reaction event ID - reactionKey := kvstore.BuildMatrixReactionKey(redactedEventID) + reactionKey := kvstore.BuildMatrixReactionKey(b.serverID, redactedEventID) reactionInfoBytes, err := b.kvstore.Get(reactionKey) if err != nil { b.logger.LogWarn("Cannot find stored Matrix reaction mapping", "matrix_event_id", redactedEventID, "redaction_event_id", redactionEventID) @@ -515,7 +515,7 @@ func (b *MatrixToMattermostBridge) handlePostDeletion(redactsEventID, channelID, // If channelID is provided, ensures the user is added to the team associated with that channel func (b *MatrixToMattermostBridge) getOrCreateMattermostUser(matrixUserID string, channelID string) (string, error) { // Check if we already have a mapping for this Matrix user - userMapKey := kvstore.BuildMatrixUserKey(matrixUserID) + userMapKey := kvstore.BuildMatrixUserKey(b.serverID, matrixUserID) userIDBytes, err := b.kvstore.Get(userMapKey) if err == nil && len(userIDBytes) > 0 { mattermostUserID := string(userIDBytes) @@ -643,7 +643,7 @@ func (b *MatrixToMattermostBridge) getOrCreateMattermostUser(matrixUserID string // Store reverse mapping: mattermost_user_ -> matrixUserID // This mapping is critical for user lookups - treat failure as a serious issue - mattermostUserKey := kvstore.BuildMattermostUserKey(createdUser.Id) + mattermostUserKey := kvstore.BuildMattermostUserKey(b.serverID, createdUser.Id) err = b.kvstore.Set(mattermostUserKey, []byte(matrixUserID)) if err != nil { b.logger.LogError("Failed to store critical reverse user mapping", "error", err, "mattermost_user_id", createdUser.Id, "matrix_user_id", matrixUserID) @@ -713,9 +713,8 @@ func (b *MatrixToMattermostBridge) generateMattermostUsername(baseUsername strin sanitized := strings.ToLower(baseUsername) sanitized = regexp.MustCompile(`[^a-z0-9\-_]`).ReplaceAllString(sanitized, "_") - // Get the configured username prefix for this server - config := b.getConfiguration() - prefix := config.GetMatrixUsernamePrefixForServer(config.MatrixServerURL) + // Get the username prefix for this server + prefix := b.matrixUsernamePrefix() // Follow Shared Channels convention: prefix:username_sanitized username := prefix + ":" + sanitized @@ -750,7 +749,7 @@ func (b *MatrixToMattermostBridge) generateMattermostUsername(baseUsername strin // then Matrix event metadata (for Mattermost-originated events). func (b *MatrixToMattermostBridge) getPostIDFromMatrixEvent(matrixEventID, channelID string) string { // First check KV store for Matrix-originated events (O(1), no network calls) - mappingKey := kvstore.BuildMatrixEventPostKey(matrixEventID) + mappingKey := kvstore.BuildMatrixEventPostKey(b.serverID, matrixEventID) if postIDBytes, err := b.kvstore.Get(mappingKey); err == nil && len(postIDBytes) > 0 { postID := string(postIDBytes) b.logger.LogDebug("Found Mattermost post ID for Matrix-originated event in KV store", "matrix_event_id", matrixEventID, "mattermost_post_id", postID) @@ -764,7 +763,7 @@ func (b *MatrixToMattermostBridge) getPostIDFromMatrixEvent(matrixEventID, chann // storeMatrixEventPostMapping stores the mapping from Matrix event ID to Mattermost post ID // for efficient reverse lookups. This is used by both message and file sync functions. func (b *MatrixToMattermostBridge) storeMatrixEventPostMapping(matrixEventID, mattermostPostID string) { - mappingKey := kvstore.BuildMatrixEventPostKey(matrixEventID) + mappingKey := kvstore.BuildMatrixEventPostKey(b.serverID, matrixEventID) if err := b.kvstore.Set(mappingKey, []byte(mattermostPostID)); err != nil { b.logger.LogWarn("Failed to store Matrix event to post mapping", "error", err, "matrix_event_id", matrixEventID, "post_id", mattermostPostID) // Continue anyway - post was created successfully @@ -1173,7 +1172,7 @@ func (b *MatrixToMattermostBridge) syncMatrixMemberEventToMattermost(event Matri } // Check if we have a Mattermost user for this Matrix user - userMapKey := kvstore.BuildMatrixUserKey(event.Sender) + userMapKey := kvstore.BuildMatrixUserKey(b.serverID, event.Sender) userIDBytes, err := b.kvstore.Get(userMapKey) existingUserID := "" userExists := false diff --git a/server/sync_to_mattermost_test.go b/server/sync_to_mattermost_test.go index 280cfb9..33fcd3c 100644 --- a/server/sync_to_mattermost_test.go +++ b/server/sync_to_mattermost_test.go @@ -15,7 +15,7 @@ func setupGetPostIDTest(t *testing.T) (*MatrixToMattermostBridge, kvstore.KVStor plugin := setupPluginForTest() plugin.client = pluginapi.NewClient(plugin.API, nil) plugin.kvstore = NewMemoryKVStore() - plugin.matrixClient = createMatrixClientWithTestLogger(t, "", "", "") + setTestMatrixClient(plugin, createMatrixClientWithTestLogger(t, "", "", "")) plugin.initBridges() return plugin.matrixToMattermostBridge, plugin.kvstore @@ -68,7 +68,7 @@ func TestGetPostIDFromMatrixEvent_KVStorePath(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup: Store mapping if needed if tc.shouldStore { - mappingKey := kvstore.BuildMatrixEventPostKey(tc.eventID) + mappingKey := kvstore.BuildMatrixEventPostKey(testServerID, tc.eventID) err := store.Set(mappingKey, []byte(tc.storedPostID)) assert.NoError(t, err) } @@ -89,7 +89,7 @@ func TestGetPostIDFromMatrixEvent_MixedEventTypes(t *testing.T) { // Test: Matrix-originated event (should use KV store) matrixEventID := "$matrix_originated_event" matrixPostID := "post_matrix_123" - mappingKey := kvstore.BuildMatrixEventPostKey(matrixEventID) + mappingKey := kvstore.BuildMatrixEventPostKey(testServerID, matrixEventID) err := store.Set(mappingKey, []byte(matrixPostID)) assert.NoError(t, err) @@ -101,9 +101,10 @@ func TestGetPostIDFromMatrixEvent_MixedEventTypes(t *testing.T) { mattermostEventID := "$mattermost_originated_event" // Verify no KV mapping exists - mappingKey2 := kvstore.BuildMatrixEventPostKey(mattermostEventID) - _, err = store.Get(mappingKey2) - assert.Error(t, err, "Should not have KV mapping for Mattermost event") + mappingKey2 := kvstore.BuildMatrixEventPostKey(testServerID, mattermostEventID) + missing, err := store.Get(mappingKey2) + assert.NoError(t, err) + assert.Empty(t, missing, "Should not have KV mapping for Mattermost event") result2 := bridge.getPostIDFromMatrixEvent(mattermostEventID, "channel_123") assert.Equal(t, "", result2, "Mattermost-originated event should fall back to Matrix API") @@ -115,7 +116,7 @@ func TestGetPostIDFromMatrixEvent_KVStoreUpdates(t *testing.T) { eventID := "$matrix_event_update" channelID := "channel_123" - mappingKey := kvstore.BuildMatrixEventPostKey(eventID) + mappingKey := kvstore.BuildMatrixEventPostKey(testServerID, eventID) // Initially no mapping result1 := bridge.getPostIDFromMatrixEvent(eventID, channelID) @@ -152,7 +153,7 @@ func TestGetPostIDFromMatrixEvent_EdgeCases(t *testing.T) { eventID := "$event_with_empty_channel" expectedPostID := "post_123" - mappingKey := kvstore.BuildMatrixEventPostKey(eventID) + mappingKey := kvstore.BuildMatrixEventPostKey(testServerID, eventID) err := store.Set(mappingKey, []byte(expectedPostID)) assert.NoError(t, err) @@ -169,9 +170,10 @@ func TestGetPostIDFromMatrixEvent_MatrixAPIFallback(t *testing.T) { channelID := "channel_123" // Verify no KV mapping exists - mappingKey := kvstore.BuildMatrixEventPostKey(eventID) - _, err := store.Get(mappingKey) - assert.Error(t, err, "Should not have KV store mapping") + mappingKey := kvstore.BuildMatrixEventPostKey(testServerID, eventID) + missing, err := store.Get(mappingKey) + assert.NoError(t, err) + assert.Empty(t, missing, "Should not have KV store mapping") // Call function - should fall back to Matrix API and return empty (since we don't have a real Matrix server) result := bridge.getPostIDFromMatrixEvent(eventID, channelID) diff --git a/server/testhelpers_test.go b/server/testhelpers_test.go index 71fc318..d87a8b7 100644 --- a/server/testhelpers_test.go +++ b/server/testhelpers_test.go @@ -1,6 +1,7 @@ package main import ( + "encoding/json" "fmt" "os" "sort" @@ -88,6 +89,45 @@ func createMatrixClientWithTestLogger(t *testing.T, serverURL, asToken, remoteID return matrix.NewClientWithLoggerAndRateLimit(serverURL, asToken, remoteID, "", testLogger, matrix.TestRateLimitConfig()) } +// testServerID is the deterministic serverID used to namespace per-server KV keys +// in tests. Using a fixed value keeps seeded keys and assertions predictable. +const testServerID = "testserveridtestserverid00" + +// setTestMatrixClient registers a Matrix client as the single server's client and +// caches the deterministic testServerID, so p.GetMatrixClient() and +// p.getSingleServerID() behave as they would after initMatrixClient. +func setTestMatrixClient(p *Plugin, client *matrix.Client) { + p.matrixClientsLock.Lock() + defer p.matrixClientsLock.Unlock() + p.serverID = testServerID + if p.matrixClients == nil { + p.matrixClients = make(map[string]*matrix.Client) + } + p.matrixClients[testServerID] = client +} + +// seedTestServerConfig writes a servers_config registry entry with the +// deterministic testServerID and caches it, so migrations and bridge KV +// operations use a predictable serverID. It mirrors what reconcileServerConfig +// does in production by projecting the plugin's flat configuration (when set) +// into the registry entry, so per-server reads such as the username prefix +// resolve to the configured values. +func seedTestServerConfig(p *Plugin) { + entry := kvstore.ServerConfig{ServerID: testServerID, Enabled: true, RemoteID: p.remoteID} + if cfg := p.configuration; cfg != nil { + entry.ServerURL = cfg.MatrixServerURL + entry.ServerName = cfg.MatrixServerName + entry.ASToken = cfg.MatrixASToken + entry.HSToken = cfg.MatrixHSToken + entry.UsernamePrefix = cfg.GetMatrixUsernamePrefix() + } + data, _ := json.Marshal([]kvstore.ServerConfig{entry}) + _ = p.kvstore.Set(kvstore.KeyServersConfig, data) + p.matrixClientsLock.Lock() + p.serverID = testServerID + p.matrixClientsLock.Unlock() +} + // TestMatrixClientTestLogger verifies that matrix client uses test logger correctly func TestMatrixClientTestLogger(t *testing.T) { // Create a matrix client with test logger @@ -124,8 +164,9 @@ func setupTestPlugin(t *testing.T, matrixContainer *matrixtest.Container) *TestS plugin.postTracker = NewPostTracker(DefaultPostTrackerMaxEntries) // Reuse the container's Matrix client to share rate limiting state - // This prevents rate limit conflicts between container setup and plugin operations - plugin.matrixClient = matrixContainer.Client + // This prevents rate limit conflicts between container setup and plugin operations. + // This also caches the deterministic testServerID used to namespace KV keys. + setTestMatrixClient(plugin, matrixContainer.Client) config := &configuration{ MatrixServerURL: matrixContainer.ServerURL, @@ -186,9 +227,10 @@ func setupBasicMocks(api *plugintest.API, testUserID string) { } // setupTestKVData sets up initial test data in the KV store -func setupTestKVData(kvstore kvstore.KVStore, testChannelID, testRoomID string) { - // Set up channel mapping - _ = kvstore.Set("channel_mapping_"+testChannelID, []byte(testRoomID)) +func setupTestKVData(store kvstore.KVStore, testChannelID, testRoomID string) { + // Set up channel mapping in the server-scoped value shape used since v3 + mappingValue, _ := kvstore.BuildSingleChannelMapping(testServerID, testRoomID) + _ = store.Set(kvstore.BuildChannelMappingKey(testChannelID), mappingValue) // Ghost users and ghost rooms are intentionally not set up here // to trigger creation during tests, which validates the creation logic @@ -266,7 +308,11 @@ func (m *MemoryKVStore) Get(key string) ([]byte, error) { copy(result, data) return result, nil } - return nil, errors.New("key not found") + // Match the production plugin KV API: a missing key returns (nil, nil), not an + // error. Only real backend failures produce an error. Callers distinguish + // "absent" via empty data, and code like getServers relies on this to tell a + // missing registry apart from a genuine read failure. + return nil, nil } // Set stores a key-value pair in the KV store. @@ -376,10 +422,14 @@ func TestMemoryKVStore(t *testing.T) { t.Errorf("Expected 'test-value', got '%s'", string(value)) } - // Test Get non-existent key - _, err = store.Get("non-existent") - if err == nil { - t.Error("Expected error for non-existent key") + // Test Get non-existent key: matches the production plugin KV API, which + // returns (nil, nil) for a missing key rather than an error. + value, err = store.Get("non-existent") + if err != nil { + t.Errorf("Expected no error for missing key, got %v", err) + } + if len(value) != 0 { + t.Errorf("Expected empty value for missing key, got '%s'", string(value)) } // Test Delete @@ -388,9 +438,12 @@ func TestMemoryKVStore(t *testing.T) { t.Errorf("Expected no error, got %v", err) } - _, err = store.Get("test-key") - if err == nil { - t.Error("Expected error for deleted key") + value, err = store.Get("test-key") + if err != nil { + t.Errorf("Expected no error for deleted key, got %v", err) + } + if len(value) != 0 { + t.Errorf("Expected empty value for deleted key, got '%s'", string(value)) } } diff --git a/server/thread_mapping_test.go b/server/thread_mapping_test.go index d95dfd1..ae43413 100644 --- a/server/thread_mapping_test.go +++ b/server/thread_mapping_test.go @@ -152,13 +152,13 @@ func (suite *ThreadMappingIntegrationTestSuite) SetupTest() { suite.plugin.postTracker = NewPostTracker(DefaultPostTrackerMaxEntries) // Create Matrix client - suite.plugin.matrixClient = createMatrixClientWithTestLogger( + setTestMatrixClient(suite.plugin, createMatrixClientWithTestLogger( suite.T(), suite.matrixContainer.ServerURL, suite.matrixContainer.ASToken, suite.plugin.remoteID, - ) - suite.plugin.matrixClient.SetServerDomain(suite.matrixContainer.ServerDomain) + )) + suite.plugin.GetMatrixClient().SetServerDomain(suite.matrixContainer.ServerDomain) // Set up configuration config := &configuration{ diff --git a/server/user_remote_detection_test.go b/server/user_remote_detection_test.go index 7305dcc..fb66382 100644 --- a/server/user_remote_detection_test.go +++ b/server/user_remote_detection_test.go @@ -57,13 +57,13 @@ func (suite *UserRemoteDetectionIntegrationTestSuite) SetupTest() { suite.plugin.postTracker = NewPostTracker(DefaultPostTrackerMaxEntries) // Create Matrix client - suite.plugin.matrixClient = createMatrixClientWithTestLogger( + setTestMatrixClient(suite.plugin, createMatrixClientWithTestLogger( suite.T(), suite.matrixContainer.ServerURL, suite.matrixContainer.ASToken, suite.plugin.remoteID, - ) - suite.plugin.matrixClient.SetServerDomain(suite.matrixContainer.ServerDomain) + )) + suite.plugin.GetMatrixClient().SetServerDomain(suite.matrixContainer.ServerDomain) // Set up configuration config := &configuration{ @@ -74,6 +74,10 @@ func (suite *UserRemoteDetectionIntegrationTestSuite) SetupTest() { } suite.plugin.configuration = config + // Project the flat config into the server registry, as reconcileServerConfig + // does at activation, so the per-server username prefix resolves to "testmatrix". + seedTestServerConfig(suite.plugin) + // Initialize the logger (required before initBridges) suite.plugin.logger = &testLogger{t: suite.T()} @@ -197,7 +201,7 @@ func (suite *UserRemoteDetectionIntegrationTestSuite) TestRealMatrixUserInteract t.Logf("✓ Created ghost user: %s", ghostUserID) // Join the test room as the ghost user - err = suite.plugin.matrixClient.JoinRoomAsUser(suite.testRoomID, ghostUserID) + err = suite.plugin.GetMatrixClient().JoinRoomAsUser(suite.testRoomID, ghostUserID) assert.NoError(t, err, "Ghost user should be able to join room") // Send a message as the ghost user to demonstrate real Matrix operations @@ -206,14 +210,14 @@ func (suite *UserRemoteDetectionIntegrationTestSuite) TestRealMatrixUserInteract GhostUserID: ghostUserID, Message: "Hello from ghost user for loop prevention test", } - response, err := suite.plugin.matrixClient.SendMessage(messageReq) + response, err := suite.plugin.GetMatrixClient().SendMessage(messageReq) assert.NoError(t, err, "Should be able to send message as ghost user") assert.NotEmpty(t, response.EventID, "Should receive event ID") t.Logf("✓ Ghost user %s sent message with event ID: %s", ghostUserID, response.EventID) // Test: Get the ghost user's profile to simulate what the bridge would do - profile, err := suite.plugin.matrixClient.GetUserProfile(ghostUserID) + profile, err := suite.plugin.GetMatrixClient().GetUserProfile(ghostUserID) assert.NoError(t, err, "Should be able to get ghost user profile") assert.NotNil(t, profile, "Profile should not be nil") @@ -374,12 +378,6 @@ func TestDefaultUsernamePrefix(t *testing.T) { t.Logf("✓ Default prefix: %s", DefaultMatrixUsernamePrefix) t.Logf("✓ Custom prefix: %s", prefix) - - // Test server-specific prefix method (for future extensibility) - serverPrefix := config.GetMatrixUsernamePrefixForServer("https://matrix.example.com") - assert.Equal(t, "customprefix", serverPrefix, "Server-specific prefix should return same as global for now") - - t.Logf("✓ Server-specific prefix: %s", serverPrefix) } // TestBasicRemoteDetectionLogic tests the basic logic without requiring Matrix server diff --git a/spec/2026-07-16-multi-matrix-server-support-phase-1.md b/spec/2026-07-16-multi-matrix-server-support-phase-1.md new file mode 100644 index 0000000..6e731b7 --- /dev/null +++ b/spec/2026-07-16-multi-matrix-server-support-phase-1.md @@ -0,0 +1,133 @@ +# Multi-Matrix-Server Support — Phase 1: Config registry + client registry + KV v3 migration + +- **Jira:** [MM-64622](https://mattermost.atlassian.net/browse/MM-64622) (Epic: MM-64621) +- **Branch:** `feat/multiple-server-support` +- **Status:** Ready to implement +- **Depends on:** nothing +- **Shippable independently:** yes (no user-visible change) + +## Objective + +Restructure the plugin internals so that a single Matrix homeserver is represented +as a **one-element registry keyed by a stable, opaque `serverID`**, and namespace all +per-server KV keys by that `serverID`. This is the foundation every later phase builds on. + +**Hard requirement: zero behavior change.** The flat `plugin.json` fields remain the +source of truth for the single configured server. No REST API, no UI, no routing, +no slash-command changes in this phase. + +## Key architectural decisions (from ticket) + +- `serverID` is the stable join key: **derived deterministically from the hostname of the + Matrix server URL**, not a random `model.NewId()`. It stays constant as long as the + hostname is unchanged. Determinism is deliberate: a server re-created with the same URL + **re-adopts its namespaced KV records** instead of orphaning them (recovery from an + accidentally deleted registry entry). Recovery holds while an independent source of the URL + survives (Phase 1: `plugin.json`); once the registry is the sole store (Phase 4+), the + payoff is that *re-adding by the same URL* re-adopts records. A legitimate hostname change + re-derives a new ID and orphans the old records (logged as a warning; full re-keying is out + of scope for Phase 1). +- Config becomes a managed registry stored in KV (`servers_config`, JSON array), but in + Phase 1 it is still _derived from_ the flat `plugin.json` config on every change. +- `matrix.Client` is already fully server-scoped (`server/matrix/client.go:182-199`), so the + single `p.matrixClient` becomes `map[serverID]*matrix.Client`. +- `channel_mapping_` keeps its key but its value becomes a **list** of + `(serverID, roomID)` entries (length 1 enforced now) to keep future fan-out cheap. +- Global master `enable_sync` stays; each server entry also carries an `Enabled` flag + (populated but not yet independently toggle-able via UI until Phase 5). + +## `serverID` + +- Derived from the `ServerConfig.ServerURL` field (hostname only), never from + `configuration.go` directly — so the same derivation works when Phase 4 feeds the URL from + the REST POST body. Reuses `matrix.ExtractServerDomain` to normalize (strip scheme, port, + path; case-folded). +- Format: `base32(sha256(hostname)[:16])[:26]` using Mattermost's ID alphabet — a 26-char + drop-in for `model.NewId()`, so no KV key-length regression. +- Single derivation site: `deriveServerID` in `server/servers.go`, called from + `reconcileServerConfig` (the v3 migration reaches it through the same function). + +## Data model + +```go +// server-side registry entry (persisted as JSON in KV under "servers_config") +type ServerConfig struct { + ServerID string `json:"server_id"` // stable, derived from hostname (see deriveServerID) + ServerURL string `json:"server_url"` + ServerName string `json:"server_name"` // Matrix ID domain (may be empty -> discovery) + ASToken string `json:"as_token"` + HSToken string `json:"hs_token"` + UsernamePrefix string `json:"username_prefix"` + Enabled bool `json:"enabled"` + RemoteID string `json:"remote_id"` // shared-channels remote; Phase 1 = the single global remoteID +} + +// value shape for channel_mapping_ (length 1 in Phase 1) +type ChannelServerMapping struct { + ServerID string `json:"server_id"` + RoomID string `json:"room_id"` +} +``` + +New KV key: `servers_config` (single JSON array). Global settings remaining flat in +`plugin.json`: `enable_sync`, `rate_limiting_mode`. + +## KV key namespacing + +Add a `serverID` dimension to these builders in `server/store/kvstore/constants.go` +(format: `_`): + +- `matrix_user_` → `BuildMatrixUserKey(serverID, matrixUserID)` +- `mattermost_user_` → `BuildMattermostUserKey(serverID, mmUserID)` +- `ghost_user_` → `BuildGhostUserKey(serverID, mmUserID)` +- `ghost_room_` → `BuildGhostRoomKey(serverID, mmUserID, roomID)` +- `matrix_event_post_` → `BuildMatrixEventPostKey(serverID, eventID)` +- `matrix_reaction_` → `BuildMatrixReactionKey(serverID, reactionEventID)` +- `room_mapping_` → `BuildRoomMappingKey(serverID, roomIdentifier)` +- `channel_mapping_`: **key unchanged**, value becomes `[]ChannelServerMapping`. + +Fix the two hardcoded string literals that bypass the constants: + +- `server/matrix_util.go:14` (`"ghost_user_" + ...`) +- `server/bridge_utils.go:287` (`"matrix_user_" + ...`) + +## Files to change + +- `server/store/kvstore/constants.go` — bump `CurrentKVStoreVersion` 2 → 3; new key builders; new `servers_config` key. +- `server/configuration.go` — reconcile flat config → single registry entry (stable `serverID`); make `GetMatrixUsernamePrefixForServer(serverID)` resolve per-entry. +- `server/plugin.go` — replace `matrixClient` with `matrixClients map[string]*matrix.Client`; add `getMatrixClient(serverID)`, `getSingleServerID()`; update `initMatrixClient`, `initBridges`, `GetMatrixClient`. +- `server/bridge_utils.go` — hold the registry + resolver; route existing calls to the single server. +- `server/migrations.go` — add `runMigrationToVersion3` (mint serverID, rekey all namespaced keys in batches, convert channel_mapping values). +- `server/sync_to_matrix.go`, `server/sync_to_mattermost.go`, `server/matrix_webhook.go`, `server/command/command.go` — update KV read/write sites to pass `serverID`. +- Tests + mocks (`server/mocks/*`, `server/command/mocks/*`). + +## Tasks + +1. Add `ServerConfig`, `ChannelServerMapping`, `servers_config` key, and new key builders. +2. Introduce the client registry on `Plugin` and the resolver on `BridgeUtils`; keep a + single-server convenience accessor. +3. Reconcile flat config into the one-element registry on `OnConfigurationChange`, keeping + `serverID` stable across restarts and config edits. +4. Thread `serverID` through every namespaced KV read/write. +5. Implement v3 migration (deterministic, idempotent). +6. Update/extend tests; run full check. + +## Testing + +- Migration v3: fresh install (no prior keys), single-server upgrade from v2, and + re-run/idempotency (running v3 twice is a no-op). +- Existing unit + integration suites still green with new key formats. +- `make check-style` and `make test` (Go + webapp) pass. Webapp untouched. + +## Out of scope (later phases) + +Inbound/outbound routing, per-server shared-channels registration, set-based loop +prevention, REST API, admin UI, slash-command targeting, `min_server_version` bump, +server-deletion semantics. + +## Acceptance criteria + +- A v2 install upgrades cleanly to v3; all mappings attributed to one minted `serverID`. +- No functional/behavioral change for an operator with one server configured. +- `servers_config` holds exactly one entry after migration. +- Flat `plugin.json` fields remain intact (rollback window preserved for one release). diff --git a/spec/2026-07-16-multi-matrix-server-support-phase-2.md b/spec/2026-07-16-multi-matrix-server-support-phase-2.md new file mode 100644 index 0000000..f5d2673 --- /dev/null +++ b/spec/2026-07-16-multi-matrix-server-support-phase-2.md @@ -0,0 +1,67 @@ +# Multi-Matrix-Server Support — Phase 2: Inbound routing (hs_token → serverID) + +- **Jira:** [MM-64622](https://mattermost.atlassian.net/browse/MM-64622) +- **Branch:** `feat/multiple-server-support` +- **Status:** Blocked on Phase 1 +- **Depends on:** Phase 1 (config registry, client registry, namespaced KV) +- **Shippable independently:** yes (core routing; still safe with one server) + +## Objective + +Route **inbound** Matrix Application Service traffic to the correct server. Identify the +originating homeserver by its `hs_token`, resolve the `serverID`, and perform all lookups +in that server's KV namespace. + +## Key architectural decision (from ticket #4) + +An AS appends the fixed suffix `/_matrix/app/v1/transactions/{txnId}` to the registration +`url`, so a discriminator cannot sit mid-path. **Give each server a unique `hs_token`** and +match the presented bearer token against every server's token to resolve `serverID` — zero +URL restructuring. (Fallback if needed: put `serverID` as the first path segment via the +registration `url` base.) + +## Tasks + +1. **Auth middleware** (`server/api.go:44-73`): replace the single-token compare with a + constant-time match of the presented `Bearer` token against each server's `HSToken`. + On match, resolve `serverID` and inject it into the request context; 401 on no match. + Keep the `enable_sync` master check plus per-server `Enabled` check. +2. **Transaction dedup** (`server/matrix_webhook.go:39-42`): change the process-global + `processedTransactions` map keyed by `txnID` to be keyed by `(serverID, txnID)` — Matrix + `txnID`s are only unique per homeserver. +3. **Event routing** (`server/matrix_webhook.go:166-243`): pass `serverID` into + `processMatrixEvent`; look up `room_mapping__`; fallback room-state + lookup uses that server's client from the registry. +4. **Ghost-user detection** (`isGhostUser`, `matrix_webhook.go:246-266`): check the suffix + against the resolved server's domain, not the single config. +5. **Inbound persistence** (`server/sync_to_mattermost.go`): all `matrix_user_`, + `mattermost_user_`, `matrix_event_post_`, `matrix_reaction_` reads/writes use `serverID`; + store post property `matrix_event_id_` for the resolved server. + +## Files to change + +`server/api.go`, `server/matrix_webhook.go`, `server/sync_to_mattermost.go`, +`server/bridge_utils.go` (server-aware helpers), plus tests. + +## Testing + +- Two-server unit tests: transactions with distinct `hs_token`s route to distinct + namespaces; colliding `txnID`s across servers are NOT deduped against each other. +- Single-server behavior unchanged. +- Unknown//invalid token → 401. + +## Out of scope + +Outbound client selection (Phase 3), per-server registration (see below), UI, REST API. + +## Open questions / risks + +- Per-server shared-channels registration (unique `remoteID` per server) is a prerequisite + for correct inbound loop attribution; coordinate the loop-prevention change with Phase 3. +- Confirm the auth middleware’s token scan is constant-time and bounded (few servers). + +## Acceptance criteria + +- Inbound events from server A never read/write server B's namespace. +- Duplicate detection is per `(serverID, txnID)`. +- One-server installs behave exactly as before. diff --git a/spec/2026-07-16-multi-matrix-server-support-phase-3.md b/spec/2026-07-16-multi-matrix-server-support-phase-3.md new file mode 100644 index 0000000..d7a05a4 --- /dev/null +++ b/spec/2026-07-16-multi-matrix-server-support-phase-3.md @@ -0,0 +1,68 @@ +# Multi-Matrix-Server Support — Phase 3: Outbound routing + per-server registration + +- **Jira:** [MM-64622](https://mattermost.atlassian.net/browse/MM-64622) +- **Branch:** `feat/multiple-server-support` +- **Status:** Blocked on Phase 1 (and coordinated with Phase 2) +- **Depends on:** Phase 1; loop-prevention change coordinated with Phase 2 +- **Shippable independently:** yes (completes the core; phases 1–3 are the core) + +## Objective + +Route **outbound** Mattermost → Matrix traffic to the correct homeserver, selecting the +Matrix client by the channel→server mapping, and register the plugin per Matrix server so +loop prevention and sync cursors are per-remote. + +## Tasks + +1. **Client selection** (`server/sync_to_matrix.go`): resolve `(serverID, roomID)` from + `channel_mapping_` (the Phase 1 list value), pick `matrixClients[serverID]`, + and send through it. Same for attachment sync and DM-room creation in `hooks.go`. +2. **Per-server ghost/user builders**: ghost user IDs, bridge aliases, and username + generation must bind to the target server's domain/prefix + (`GetMatrixUsernamePrefixForServer(serverID)`), not the single config. +3. **Fix `extractUsernameFromMatrixUserID`** (`server/sync_to_mattermost.go:695-708`): retain + the server component instead of discarding it, so round-tripping `@user:server` is + server-aware. +4. **Per-server shared-channels registration** (`server/plugin.go:183-219`, ticket #5): + call `RegisterPluginForSharedChannels` **once per Matrix server** with a distinct + `SiteURL` (e.g. `https://`), storing each returned `remoteID` on the + server's registry entry. Re-registration by the same SiteURL is idempotent. +5. **Set-based loop prevention** (`server/hooks.go:39,52,108,274`): replace + `x.GetRemoteID() == p.remoteID` with membership in the set of the plugin's own + `remoteID`s (one per server). + +## Data / config + +- `ServerConfig.RemoteID` becomes populated per server (Phase 1 seeded it with the single + global remote; here it becomes genuinely per-server). +- Maintain a fast `map[remoteID]serverID` lookup for loop attribution. + +## Files to change + +`server/sync_to_matrix.go`, `server/sync_to_mattermost.go`, `server/hooks.go`, +`server/plugin.go`, `server/bridge_utils.go`, `server/matrix_util.go`, plus tests. + +## `min_server_version` + +Confirm the earliest Mattermost server version that ships the `SiteURL` field on +`RegisterPluginOpts` (per-remote registration) and bump `plugin.json` `min_server_version` +accordingly (currently `10.7.1`). + +## Testing + +- Two-server: a post in a channel mapped to server A sends only via A's client; a remote + post from A is not re-synced (its `remoteID` is in the own-set), while a genuinely local + post still syncs. +- `extractUsernameFromMatrixUserID` round-trips server-qualified IDs. +- Single-server behavior unchanged. + +## Out of scope + +REST API, admin UI, slash-command targeting. + +## Acceptance criteria + +- Outbound posts/reactions/files/profile-images target the mapped server only. +- Distinct `remoteID` per server; loop prevention correct across servers. +- Core (phases 1–3) supports N servers end-to-end when servers_config has N entries + (even though only phases 4–6 make N configurable/usable by an admin). diff --git a/spec/2026-07-16-multi-matrix-server-support-phase-4.md b/spec/2026-07-16-multi-matrix-server-support-phase-4.md new file mode 100644 index 0000000..01ec953 --- /dev/null +++ b/spec/2026-07-16-multi-matrix-server-support-phase-4.md @@ -0,0 +1,66 @@ +# Multi-Matrix-Server Support — Phase 4: REST API + server-side registration generation + +- **Jira:** [MM-64622](https://mattermost.atlassian.net/browse/MM-64622) +- **Branch:** `feat/multiple-server-support` +- **Status:** Blocked on Phase 1 (functionally needs 1–3 for the servers to do anything) +- **Depends on:** Phase 1 (registry); best after Phases 2–3 +- **Shippable independently:** yes (API only; UI in Phase 5) + +## Objective + +Expose the `servers_config` registry through a plugin REST API so servers can be listed, +added, edited, and removed, and generate each server's Application Service registration +file server-side (replacing the browser DOM-scraping component). + +## REST API (under `/api/v1`, `MattermostAuthorizationRequired` + sysadmin check) + +- `GET /servers` — list servers (tokens redacted/masked). +- `POST /servers` — add a server; **derive `serverID` deterministically from the URL + hostname** (`deriveServerID`, Phase 1); persist; (re)register shared-channels remote; + rebuild client registry. Because the ID is derived, a re-added server re-adopts its + orphaned KV records, and a duplicate URL resolves to an existing `serverID` — treat that as + an idempotent add/update, not a second entry. +- `PUT /servers/{serverID}` — edit URL/name/tokens/prefix/enabled. +- `DELETE /servers/{serverID}` — remove a server (see deletion semantics). +- `GET /servers/{serverID}/registration` — download that server's AS registration YAML, + built server-side from its URL/domain/tokens (see below). +- `POST /servers/{serverID}/test` — connectivity/health check via that server's client. + +All writes go through a single serialized path that updates `servers_config` and refreshes +the in-memory client registry atomically. + +## Server-side registration generation (ticket #7) + +Move YAML generation out of `webapp/.../registration_download` into the server. Build from +the server entry: `id`, `url` (SiteURL + plugin path), `as_token`, `hs_token`, +`sender_localpart`, and namespaces derived from the server's domain +(`@_mattermost_.*:`, alias/room regexes). Keep the emitted YAML byte-compatible +with today's file for the existing single server. + +## Deletion semantics (resolved) + +Mirror Mattermost Connected Workspaces: removing a server **stops syncing** and tears down +its shared-channels remote; it does **not** delete channel content and does not hard-block. +Provide a guard/warning when channels are still mapped, and clean up (or orphan-mark) that +server's namespaced KV keys. Finalize exact cleanup batch here. + +## Files to change + +`server/api.go` (routes + handlers), new `server/servers_api.go` (or similar), +`server/configuration.go` (registry mutation helpers), `plugin.json` (drop/mark the +registration-download custom setting once server-side generation lands), tests. + +## Testing + +- CRUD happy paths + validation (dupe URL, malformed URL, missing tokens). +- Registration YAML for the migrated single server is byte-identical to the legacy output. +- AuthZ: non-admins are rejected; tokens are never returned in plaintext by `GET`. + +## Out of scope + +Admin UI (Phase 5), slash-command targeting (Phase 6). + +## Acceptance criteria + +- Admin can fully manage servers over REST without editing `plugin.json`. +- Per-server registration file downloads correctly and matches homeserver expectations. diff --git a/spec/2026-07-16-multi-matrix-server-support-phase-5.md b/spec/2026-07-16-multi-matrix-server-support-phase-5.md new file mode 100644 index 0000000..59d242a --- /dev/null +++ b/spec/2026-07-16-multi-matrix-server-support-phase-5.md @@ -0,0 +1,55 @@ +# Multi-Matrix-Server Support — Phase 5: Admin UI (custom server-management console) + +- **Jira:** [MM-64622](https://mattermost.atlassian.net/browse/MM-64622) +- **Branch:** `feat/multiple-server-support` +- **Status:** Blocked on Phase 4 (REST API) and on Figma designs +- **Depends on:** Phase 4 +- **Shippable independently:** yes (UI over existing API) + +## Objective + +Replace the flat, single-server `plugin.json` settings UI with a custom System Console +component that manages the list of Matrix servers (add / edit / enable / remove / download +registration / test) backed by the Phase 4 REST API. + +## ⚠️ Design dependency — action required before building + +The Jira remote link points to Figma file +`Matrix-support-for-connected-workspaces` (`OUH75QZFSY0PvDHUlbToIc`), but its **main branch +contains only a project Cover page — no admin-UI frames**. The actual designs are expected +to live on a **Figma branch** of that file. + +**Blocker:** obtain the branch/frame-specific URL, i.e. either +`https://www.figma.com/design//branch//...` or a main-file URL with a +`?node-id=` for the admin screens. Reconcile the implementation with those frames +before writing UI code (ticket calls this out as a risk). + +## Scope + +- New React admin-console setting component rendering the server list and add/edit forms. +- Retire the flat single-server fields and the DOM-scraping `registration_download` + component (superseded by the Phase 4 server-side endpoint). +- Global `enable_sync` master toggle remains; per-server `Enabled` toggle in the list. +- Global settings (`rate_limiting_mode`) stay as standard settings. + +## Files to change + +`webapp/src/components/admin_console_settings/*` (new server-manager component; +remove/replace `registration_download` and `homeserver_config`), `webapp/src/index.tsx` +(registration), `plugin.json` (settings_schema restructure), webapp tests. + +## Testing + +- Component tests for list/add/edit/delete/enable and registration download. +- Manual QA against a running server with 2 homeservers configured. +- Verify graceful behavior on API errors and token redaction. + +## Out of scope + +Slash-command targeting (Phase 6). + +## Acceptance criteria + +- Admin manages N servers entirely from the System Console. +- UI matches the approved Figma frames. +- Single-server upgrade presents the migrated server pre-populated in the list. diff --git a/spec/2026-07-16-multi-matrix-server-support-phase-6.md b/spec/2026-07-16-multi-matrix-server-support-phase-6.md new file mode 100644 index 0000000..0e64b13 --- /dev/null +++ b/spec/2026-07-16-multi-matrix-server-support-phase-6.md @@ -0,0 +1,46 @@ +# Multi-Matrix-Server Support — Phase 6: Slash-command server targeting + +- **Jira:** [MM-64622](https://mattermost.atlassian.net/browse/MM-64622) +- **Branch:** `feat/multiple-server-support` +- **Status:** Blocked on Phase 1 (needs 2–3 for real routing) +- **Depends on:** Phases 1–3 +- **Shippable independently:** yes + +## Objective + +Give the `/matrix` slash commands server awareness so operators can target a specific +Matrix homeserver, while keeping single-server behavior unchanged. + +## Tasks + +1. **`/matrix map`** (`server/command/command.go:332-492`): infer the target server from the + room domain (`#alias:server` / `!id:server`) by matching against configured servers' + domains; error clearly if ambiguous/unknown. Store the mapping with the resolved + `serverID` (Phase 1 list value). +2. **`/matrix create`** (`command.go:578-668`): add a `--server ` flag to + choose the homeserver; default to the single server when only one is configured. +3. **`/matrix servers`**: new subcommand listing configured servers (id, domain, enabled, + mapped-channel count). Add to dispatch (`command.go:762-853`) and autocomplete + (`command.go:266-293`). +4. **`/matrix status` / `list`**: show per-server breakdown. +5. Update the `Configuration`/`PluginAccessor` command interfaces + (`command.go:18-36`) to expose per-server lookups and the client registry. + +## Files to change + +`server/command/command.go`, command mocks, tests. + +## Testing + +- Single server: all existing commands behave identically (no `--server` needed). +- Multi server: `map` infers server from domain; `create --server` targets correctly; + `servers` lists all; ambiguous input errors cleanly. + +## Out of scope + +Nothing further — this completes the epic's listed phases. + +## Acceptance criteria + +- Every `/matrix` subcommand works with N servers and is unchanged with one server. +- `/matrix servers` accurately reflects `servers_config`. diff --git a/webapp/package-lock.json b/webapp/package-lock.json index ea4c9f8..70b17dd 100644 --- a/webapp/package-lock.json +++ b/webapp/package-lock.json @@ -4,7 +4,6 @@ "requires": true, "packages": { "": { - "name": "webapp", "dependencies": { "core-js": "3.26.0", "mattermost-redux": "10.8.0", From df81b757545781cc3799ff8550f21c291e30ef98 Mon Sep 17 00:00:00 2001 From: Felipe Martin Date: Mon, 20 Jul 2026 10:58:45 +0200 Subject: [PATCH 2/2] chore: add element matrix client to docker compose --- README.md | 20 ++++++++++++++++++++ docker-compose.yml | 10 ++++++++++ docker/element-config.json | 13 +++++++++++++ 3 files changed, 43 insertions(+) create mode 100644 docker/element-config.json diff --git a/README.md b/README.md index 6c0eae0..d63c396 100644 --- a/README.md +++ b/README.md @@ -125,12 +125,32 @@ For local development and testing, you can run a Matrix Synapse server using Doc 3. The Matrix server will be available at `http://localhost:8888` +### Accessing the Web Chat Interface (Element) + +Synapse is only a homeserver and has no built-in chat UI. The Docker Compose stack +includes an [Element Web](https://element.io/) client for testing: + +1. Start the Element service (included in `docker-compose up -d`, or start it alone): + + ```bash + docker-compose up -d element + ``` + +2. Open `http://localhost:8880` in your browser. + +3. It is pre-configured to use the local homeserver (`http://localhost:8888`), so you + can register or sign in with a test user directly. Registration is enabled for + development, so you can create new users from the login screen. + +Element's configuration lives in `docker/element-config.json`. + ### Configuration Notes - The Synapse server is configured to use PostgreSQL as the database - Registration is enabled for development purposes - App service configuration is loaded from `docker/mattermost-bridge-registration.yaml` - Room list publication is restricted to the bridge user only +- An Element Web client is available at `http://localhost:8880` for manual testing ### Stopping the Services diff --git a/docker-compose.yml b/docker-compose.yml index 5b63788..1a69bcc 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -30,6 +30,16 @@ services: - synapse-init - postgres + element: + image: vectorim/element-web:latest + restart: unless-stopped + ports: + - "8880:80" + volumes: + - ./docker/element-config.json:/app/config.json:ro + depends_on: + - synapse + postgres: image: postgres:14 restart: unless-stopped diff --git a/docker/element-config.json b/docker/element-config.json new file mode 100644 index 0000000..8f2c043 --- /dev/null +++ b/docker/element-config.json @@ -0,0 +1,13 @@ +{ + "default_server_config": { + "m.homeserver": { + "base_url": "http://localhost:8888", + "server_name": "localhost" + } + }, + "disable_custom_urls": false, + "disable_guests": false, + "brand": "Element (dev)", + "default_country_code": "US", + "show_labs_settings": true +}