feat: implement comprehensive room validation and admin-only command access

- Add RoomExists and GetRoomMapping methods to Bridge interface
- Implement XMPP room existence checking using disco#info queries (XEP-0030)
- Add room validation in BridgeManager to prevent duplicate mappings and invalid rooms
- Enhance XMPP client with CheckRoomExists method and comprehensive logging
- Implement admin-only access control for all bridge commands
- Add user-friendly error messages with actionable troubleshooting steps
- Update doctor command with room existence testing and pre-join validation
- Add SimpleLogger implementation for standalone command usage

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

server/bridge/manager.go

@@ -245,6 +245,39 @@ func (m *Manager) OnChannelMappingCreated(req model.ChannelMappingRequest) error
 		return fmt.Errorf("bridge '%s' is not connected", req.BridgeName)
 	}
 
+	// NEW: Check if room already mapped to another channel
+	existingChannelID, err := bridge.GetRoomMapping(req.BridgeRoomID)
+	if err != nil {
+		m.logger.LogError("Failed to check room mapping", "bridge_room_id", req.BridgeRoomID, "error", err)
+		return fmt.Errorf("failed to check room mapping: %w", err)
+	}
+	if existingChannelID != "" {
+		m.logger.LogWarn("Room already mapped to another channel", 
+			"bridge_room_id", req.BridgeRoomID, 
+			"existing_channel_id", existingChannelID, 
+			"requested_channel_id", req.ChannelID)
+		return fmt.Errorf("room '%s' is already mapped to channel '%s'", req.BridgeRoomID, existingChannelID)
+	}
+
+	// NEW: Check if room exists on target bridge
+	roomExists, err := bridge.RoomExists(req.BridgeRoomID)
+	if err != nil {
+		m.logger.LogError("Failed to check room existence", "bridge_room_id", req.BridgeRoomID, "error", err)
+		return fmt.Errorf("failed to check room existence: %w", err)
+	}
+	if !roomExists {
+		m.logger.LogWarn("Room does not exist on bridge", 
+			"bridge_room_id", req.BridgeRoomID, 
+			"bridge_name", req.BridgeName)
+		return fmt.Errorf("room '%s' does not exist on %s bridge", req.BridgeRoomID, req.BridgeName)
+	}
+
+	m.logger.LogDebug("Room validation passed", 
+		"bridge_room_id", req.BridgeRoomID, 
+		"bridge_name", req.BridgeName,
+		"room_exists", roomExists,
+		"already_mapped", false)
+
 	// Create the channel mapping on the receiving bridge
 	if err = bridge.CreateChannelMapping(req.ChannelID, req.BridgeRoomID); err != nil {
 		m.logger.LogError("Failed to create channel mapping", "channel_id", req.ChannelID, "bridge_name", req.BridgeName, "bridge_room_id", req.BridgeRoomID, "error", err)

server/bridge/mattermost/bridge.go

@@ -255,3 +255,53 @@ func (b *mattermostBridge) DeleteChannelMapping(channelID string) error {
 	b.logger.LogInfo("Deleted Mattermost channel room mapping", "channel_id", channelID, "room_id", roomID)
 	return nil
 }
+
+// RoomExists checks if a Mattermost channel exists on the server
+func (b *mattermostBridge) RoomExists(roomID string) (bool, error) {
+	if b.api == nil {
+		return false, fmt.Errorf("Mattermost API not initialized")
+	}
+
+	b.logger.LogDebug("Checking if Mattermost channel exists", "channel_id", roomID)
+
+	// Use the Mattermost API to check if the channel exists
+	channel, appErr := b.api.GetChannel(roomID)
+	if appErr != nil {
+		if appErr.StatusCode == 404 {
+			b.logger.LogDebug("Mattermost channel does not exist", "channel_id", roomID)
+			return false, nil
+		}
+		b.logger.LogError("Failed to check channel existence", "channel_id", roomID, "error", appErr)
+		return false, fmt.Errorf("failed to check channel existence: %w", appErr)
+	}
+
+	if channel == nil {
+		b.logger.LogDebug("Mattermost channel does not exist (nil response)", "channel_id", roomID)
+		return false, nil
+	}
+
+	b.logger.LogDebug("Mattermost channel exists", "channel_id", roomID, "channel_name", channel.Name)
+	return true, nil
+}
+
+// GetRoomMapping retrieves the Mattermost channel ID for a given room ID (reverse lookup)
+func (b *mattermostBridge) GetRoomMapping(roomID string) (string, error) {
+	if b.kvstore == nil {
+		return "", fmt.Errorf("KV store not initialized")
+	}
+
+	b.logger.LogDebug("Getting channel mapping for Mattermost room", "room_id", roomID)
+
+	// Look up the channel ID using the room ID as the key
+	channelIDBytes, err := b.kvstore.Get(kvstore.BuildChannelMapKey("mattermost", roomID))
+	if err != nil {
+		// No mapping found is not an error, just return empty string
+		b.logger.LogDebug("No channel mapping found for room", "room_id", roomID)
+		return "", nil
+	}
+
+	channelID := string(channelIDBytes)
+	b.logger.LogDebug("Found channel mapping for room", "room_id", roomID, "channel_id", channelID)
+
+	return channelID, nil
+}

server/bridge/xmpp/bridge.go

@@ -73,6 +73,7 @@ func (b *xmppBridge) createXMPPClient(cfg *config.Configuration) *xmppClient.Cli
 		cfg.GetXMPPResource(),
 		"", // remoteID not needed for bridge user
 		tlsConfig,
+		b.logger,
 	)
 }
 
@@ -471,3 +472,48 @@ func (b *xmppBridge) DeleteChannelMapping(channelID string) error {
 	b.logger.LogInfo("Deleted channel room mapping", "channel_id", channelID, "room_jid", roomJID)
 	return nil
 }
+
+// RoomExists checks if an XMPP room exists on the remote service
+func (b *xmppBridge) RoomExists(roomID string) (bool, error) {
+	if !b.connected.Load() {
+		return false, fmt.Errorf("not connected to XMPP server")
+	}
+
+	if b.xmppClient == nil {
+		return false, fmt.Errorf("XMPP client not initialized")
+	}
+
+	b.logger.LogDebug("Checking if XMPP room exists", "room_jid", roomID)
+
+	// Use the XMPP client to check room existence
+	exists, err := b.xmppClient.CheckRoomExists(roomID)
+	if err != nil {
+		b.logger.LogError("Failed to check room existence", "room_jid", roomID, "error", err)
+		return false, fmt.Errorf("failed to check room existence: %w", err)
+	}
+
+	b.logger.LogDebug("Room existence check completed", "room_jid", roomID, "exists", exists)
+	return exists, nil
+}
+
+// GetRoomMapping retrieves the Mattermost channel ID for a given XMPP room JID (reverse lookup)
+func (b *xmppBridge) GetRoomMapping(roomID string) (string, error) {
+	if b.kvstore == nil {
+		return "", fmt.Errorf("KV store not initialized")
+	}
+
+	b.logger.LogDebug("Getting channel mapping for XMPP room", "room_jid", roomID)
+
+	// Look up the channel ID using the room JID as the key
+	channelIDBytes, err := b.kvstore.Get(kvstore.BuildChannelMapKey("xmpp", roomID))
+	if err != nil {
+		// No mapping found is not an error, just return empty string
+		b.logger.LogDebug("No channel mapping found for room", "room_jid", roomID)
+		return "", nil
+	}
+
+	channelID := string(channelIDBytes)
+	b.logger.LogDebug("Found channel mapping for room", "room_jid", roomID, "channel_id", channelID)
+	
+	return channelID, nil
+}

server/command/command.go

@@ -54,6 +54,14 @@ func NewCommandHandler(client *pluginapi.Client, bridgeManager pluginModel.Bridg
 
 // ExecuteCommand hook calls this method to execute the commands that were registered in the NewCommandHandler function.
 func (c *Handler) Handle(args *model.CommandArgs) (*model.CommandResponse, error) {
+	// Check if user is system admin for all plugin commands
+	if !c.isSystemAdmin(args.UserId) {
+		return &model.CommandResponse{
+			ResponseType: model.CommandResponseTypeEphemeral,
+			Text:         "❌ Only system administrators can use XMPP bridge commands.",
+		}, nil
+	}
+
 	trigger := strings.TrimPrefix(strings.Fields(args.Command)[0], "/")
 	switch trigger {
 	case xmppBridgeCommandTrigger:
@@ -162,10 +170,7 @@ func (c *Handler) executeMapCommand(args *model.CommandArgs, fields []string) *m
 	
 	err = c.bridgeManager.OnChannelMappingCreated(mappingReq)
 	if err != nil {
-		return &model.CommandResponse{
-			ResponseType: model.CommandResponseTypeEphemeral,
-			Text:         fmt.Sprintf("❌ Failed to create channel mapping: %v", err),
-		}
+		return c.formatMappingError("create", roomJID, err)
 	}
 
 	return &model.CommandResponse{
@@ -212,10 +217,7 @@ func (c *Handler) executeUnmapCommand(args *model.CommandArgs) *model.CommandRes
 	
 	err = c.bridgeManager.OnChannelMappingDeleted(deleteReq)
 	if err != nil {
-		return &model.CommandResponse{
-			ResponseType: model.CommandResponseTypeEphemeral,
-			Text:         fmt.Sprintf("❌ Failed to unmap channel: %v", err),
-		}
+		return c.formatMappingError("delete", roomJID, err)
 	}
 
 	return &model.CommandResponse{
@@ -269,3 +271,85 @@ func (c *Handler) executeStatusCommand(args *model.CommandArgs) *model.CommandRe
 - Use `+"`/xmppbridge unmap`"+` to unmap this channel from an XMPP room`, statusText, mappingText),
 	}
 }
+
+// isSystemAdmin checks if the user is a system administrator
+func (c *Handler) isSystemAdmin(userID string) bool {
+	user, err := c.client.User.Get(userID)
+	if err != nil {
+		c.client.Log.Warn("Failed to get user for admin check", "user_id", userID, "error", err)
+		return false
+	}
+	
+	return user.IsSystemAdmin()
+}
+
+// formatMappingError provides user-friendly error messages for mapping operations
+func (c *Handler) formatMappingError(operation, roomJID string, err error) *model.CommandResponse {
+	errorMsg := err.Error()
+	
+	// Handle specific error cases with user-friendly messages
+	switch {
+	case strings.Contains(errorMsg, "already mapped to channel"):
+		return &model.CommandResponse{
+			ResponseType: model.CommandResponseTypeEphemeral,
+			Text: fmt.Sprintf(`❌ **Room Already Mapped**
+
+The XMPP room **%s** is already connected to another channel.
+
+**What you can do:**
+- Choose a different XMPP room that isn't already in use
+- Unmap the room from the other channel first using ` + "`/xmppbridge unmap`" + `
+- Use ` + "`/xmppbridge status`" + ` to check current mappings`, roomJID),
+		}
+		
+	case strings.Contains(errorMsg, "does not exist"):
+		return &model.CommandResponse{
+			ResponseType: model.CommandResponseTypeEphemeral,
+			Text: fmt.Sprintf(`❌ **Room Not Found**
+
+The XMPP room **%s** doesn't exist or isn't accessible.
+
+**What you can do:**
+- Check that the room JID is spelled correctly
+- Make sure the room exists on the XMPP server
+- Verify you have permission to access the room
+- Contact your XMPP administrator if needed
+
+**Example format:** room@conference.example.com`, roomJID),
+		}
+		
+	case strings.Contains(errorMsg, "not connected"):
+		return &model.CommandResponse{
+			ResponseType: model.CommandResponseTypeEphemeral,
+			Text: `❌ **Bridge Not Connected**
+
+The XMPP bridge is currently disconnected.
+
+**What you can do:**
+- Wait a moment and try again (the bridge may be reconnecting)
+- Contact your system administrator
+- Use ` + "`/xmppbridge status`" + ` to check the connection status`,
+		}
+		
+	default:
+		// Generic error message for unknown cases
+		action := "create the mapping"
+		if operation == "delete" {
+			action = "remove the mapping"
+		}
+		
+		return &model.CommandResponse{
+			ResponseType: model.CommandResponseTypeEphemeral,
+			Text: fmt.Sprintf(`❌ **Operation Failed**
+
+Unable to %s for room **%s**.
+
+**What you can do:**
+- Try the command again in a few moments
+- Use ` + "`/xmppbridge status`" + ` to check the bridge status
+- Contact your system administrator if the problem persists
+
+**Error details:** %s`, action, roomJID, errorMsg),
+		}
+	}
+}

server/model/bridge.go

@@ -133,6 +133,12 @@ type Bridge interface {
 	// DeleteChannelMapping removes a mapping between a Mattermost channel ID and a bridge room ID.
 	DeleteChannelMapping(channelID string) error
 
+	// RoomExists checks if a room/channel exists on the remote service.
+	RoomExists(roomID string) (bool, error)
+
+	// GetRoomMapping retrieves the Mattermost channel ID for a given room ID (reverse lookup).
+	GetRoomMapping(roomID string) (string, error)
+
 	// IsConnected checks if the bridge is connected to the remote service.
 	IsConnected() bool
 }

server/plugin.go

@@ -153,6 +153,7 @@ func (p *Plugin) initXMPPClient() {
 		cfg.XMPPPassword,
 		cfg.GetXMPPResource(),
 		p.remoteID,
+		p.logger,
 	)
 }
 

server/xmpp/client.go

@@ -8,8 +8,10 @@ import (
 	"fmt"
 	"time"
 
+	"github.com/mattermost/mattermost-plugin-bridge-xmpp/server/logger"
 	"mellium.im/sasl"
 	"mellium.im/xmpp"
+	"mellium.im/xmpp/disco"
 	"mellium.im/xmpp/jid"
 	"mellium.im/xmpp/muc"
 	"mellium.im/xmpp/mux"
@@ -25,6 +27,7 @@ type Client struct {
 	remoteID     string      // Plugin remote ID for metadata
 	serverDomain string      // explicit server domain for testing
 	tlsConfig    *tls.Config // custom TLS configuration
+	logger       logger.Logger // Logger for debugging
 
 	// XMPP connection
 	session       *xmpp.Session
@@ -80,7 +83,7 @@ type UserProfile struct {
 }
 
 // NewClient creates a new XMPP client.
-func NewClient(serverURL, username, password, resource, remoteID string) *Client {
+func NewClient(serverURL, username, password, resource, remoteID string, logger logger.Logger) *Client {
 	ctx, cancel := context.WithCancel(context.Background())
 	mucClient := &muc.Client{}
 	mux := mux.New("jabber:client", muc.HandleClient(mucClient))
@@ -91,6 +94,7 @@ func NewClient(serverURL, username, password, resource, remoteID string) *Client
 		password:     password,
 		resource:     resource,
 		remoteID:     remoteID,
+		logger:       logger,
 		ctx:          ctx,
 		cancel:       cancel,
 		mucClient:    mucClient,
@@ -100,8 +104,8 @@ func NewClient(serverURL, username, password, resource, remoteID string) *Client
 }
 
 // NewClientWithTLS creates a new XMPP client with custom TLS configuration.
-func NewClientWithTLS(serverURL, username, password, resource, remoteID string, tlsConfig *tls.Config) *Client {
-	client := NewClient(serverURL, username, password, resource, remoteID)
+func NewClientWithTLS(serverURL, username, password, resource, remoteID string, tlsConfig *tls.Config, logger logger.Logger) *Client {
+	client := NewClient(serverURL, username, password, resource, remoteID, logger)
 	client.tlsConfig = tlsConfig
 	return client
 }
@@ -430,3 +434,93 @@ func (c *Client) SetOnlinePresence() error {
 
 	return nil
 }
+
+// CheckRoomExists verifies if an XMPP room exists and is accessible using disco#info
+func (c *Client) CheckRoomExists(roomJID string) (bool, error) {
+	if c.session == nil {
+		return false, fmt.Errorf("XMPP session not established")
+	}
+
+	c.logger.LogDebug("Checking room existence using disco#info", "room_jid", roomJID)
+
+	// Parse and validate the room JID
+	roomAddr, err := jid.Parse(roomJID)
+	if err != nil {
+		c.logger.LogError("Invalid room JID", "room_jid", roomJID, "error", err)
+		return false, fmt.Errorf("invalid room JID: %w", err)
+	}
+
+	// Set timeout for the disco query
+	ctx, cancel := context.WithTimeout(c.ctx, 10*time.Second)
+	defer cancel()
+
+	// Perform disco#info query to the room
+	info, err := disco.GetInfo(ctx, "", roomAddr, c.session)
+	if err != nil {
+		// Check if it's a service-unavailable or item-not-found error
+		if stanzaErr, ok := err.(stanza.Error); ok {
+			c.logger.LogDebug("Received stanza error during disco#info query", 
+				"room_jid", roomJID,
+				"error_condition", string(stanzaErr.Condition),
+				"error_type", string(stanzaErr.Type))
+
+			switch stanzaErr.Condition {
+			case stanza.ServiceUnavailable, stanza.ItemNotFound:
+				c.logger.LogDebug("Room does not exist", "room_jid", roomJID, "condition", string(stanzaErr.Condition))
+				return false, nil // Room doesn't exist
+			case stanza.Forbidden:
+				c.logger.LogWarn("Access denied to room (room exists but not accessible)", "room_jid", roomJID)
+				return false, fmt.Errorf("access denied to room %s", roomJID)
+			case stanza.NotAuthorized:
+				c.logger.LogWarn("Not authorized to query room (room exists but not queryable)", "room_jid", roomJID)
+				return false, fmt.Errorf("not authorized to query room %s", roomJID)
+			default:
+				c.logger.LogError("Unexpected disco query error", "room_jid", roomJID, "condition", string(stanzaErr.Condition), "error", err)
+				return false, fmt.Errorf("disco query failed: %w", err)
+			}
+		}
+		c.logger.LogError("Disco query error", "room_jid", roomJID, "error", err)
+		return false, fmt.Errorf("disco query error: %w", err)
+	}
+
+	c.logger.LogDebug("Received disco#info response, checking for MUC features", 
+		"room_jid", roomJID,
+		"features_count", len(info.Features),
+		"identities_count", len(info.Identity))
+
+	// Verify it's actually a MUC room by checking features
+	for _, feature := range info.Features {
+		if feature.Var == muc.NS { // "http://jabber.org/protocol/muc"
+			c.logger.LogDebug("Room exists and has MUC feature", "room_jid", roomJID)
+			return true, nil
+		}
+	}
+
+	// Check for conference identity as backup verification
+	for _, identity := range info.Identity {
+		if identity.Category == "conference" {
+			c.logger.LogDebug("Room exists and has conference identity", "room_jid", roomJID, "identity_type", identity.Type)
+			return true, nil
+		}
+	}
+
+	// Log all features and identities for debugging
+	c.logger.LogDebug("Room exists but doesn't appear to be a MUC room", 
+		"room_jid", roomJID,
+		"features", func() []string {
+			var features []string
+			for _, f := range info.Features {
+				features = append(features, f.Var)
+			}
+			return features
+		}(),
+		"identities", func() []string {
+			var identities []string
+			for _, i := range info.Identity {
+				identities = append(identities, fmt.Sprintf("%s/%s", i.Category, i.Type))
+			}
+			return identities
+		}())
+
+	return false, nil
+}