Put command access checks on the command level

Checking using `settings.Allowed` is odd. Not all commands are related
to setting configuration settings. Admin commands are coming in the
future, for which this is certainly not the case.

We now do access checks early on (during command processing), so command
handlers can be clean of access checks. If we're inside of a command
handler, the user is privileged to run it.

bot/access.go

@@ -0,0 +1,37 @@
+package bot
+
+import (
+	"fmt"
+
+	"maunium.net/go/mautrix/id"
+
+	"gitlab.com/etke.cc/postmoogle/utils"
+)
+
+type accessCheckerFunc func(id.UserID, id.RoomID) (bool, error)
+
+func (b *Bot) allowAnyone(actorID id.UserID, targetRoomID id.RoomID) (bool, error) {
+	return true, nil
+}
+
+func (b *Bot) allowOwner(actorID id.UserID, targetRoomID id.RoomID) (bool, error) {
+	if !utils.Match(actorID.String(), b.allowedUsers) {
+		return false, nil
+	}
+
+	if b.noowner {
+		return true, nil
+	}
+
+	cfg, err := b.getSettings(targetRoomID)
+	if err != nil {
+		return false, fmt.Errorf("failed to retrieve settings: %v", err)
+	}
+
+	owner := cfg.Owner()
+	if owner == "" {
+		return true, nil
+	}
+
+	return owner == actorID.String(), nil
+}

bot/bot.go

@@ -22,6 +22,7 @@ type Bot struct {
 	domain                  string
 	allowedUsers            []*regexp.Regexp
 	allowedAdmins           []*regexp.Regexp
+	commands                commandList
 	rooms                   sync.Map
 	log                     *logger.Logger
 	lp                      *linkpearl.Linkpearl
@@ -38,7 +39,7 @@ func New(
 	allowedUsers []*regexp.Regexp,
 	allowedAdmins []*regexp.Regexp,
 ) *Bot {
-	return &Bot{
+	b := &Bot{
 		noowner:       noowner,
 		federation:    federation,
 		prefix:        prefix,
@@ -50,6 +51,10 @@ func New(
 		lp:            lp,
 		mu:            map[id.RoomID]*sync.Mutex{},
 	}
+
+	b.commands = b.buildCommandList()
+
+	return b
 }
 
 // Error message to the log and matrix room

bot/command.go

@@ -11,11 +11,18 @@ import (
 	"gitlab.com/etke.cc/postmoogle/utils"
 )
 
+const (
+	commandHelp      = "help"
+	commandStop      = "stop"
+	commandMailboxes = "mailboxes"
+)
+
 type (
 	command struct {
 		key           string
 		description   string
 		sanitizer     func(string) string
+		accessChecker accessCheckerFunc
 	}
 	commandList []command
 )
@@ -29,15 +36,18 @@ func (c commandList) get(key string) *command {
 	return nil
 }
 
-var commands = commandList{
+func (b *Bot) buildCommandList() commandList {
+	return commandList{
 		// special commands
 		{
-		key:         "help",
+			key:           commandHelp,
 			description:   "Show this help message",
+			accessChecker: b.allowAnyone,
 		},
 		{
-		key:         "stop",
+			key:           commandStop,
 			description:   "Disable bridge for the room and clear all configuration",
+			accessChecker: b.allowOwner,
 		},
 		{}, // delimiter
 		// options commands
@@ -45,11 +55,13 @@ var commands = commandList{
 			key:           optionMailbox,
 			description:   "Get or set mailbox of the room",
 			sanitizer:     utils.Mailbox,
+			accessChecker: b.allowOwner,
 		},
 		{
 			key:           optionOwner,
 			description:   "Get or set owner of the room",
 			sanitizer:     func(s string) string { return s },
+			accessChecker: b.allowOwner,
 		},
 		{}, // delimiter
 		{
@@ -59,6 +71,7 @@ var commands = commandList{
 				optionNoSender,
 			),
 			sanitizer:     utils.SanitizeBoolString,
+			accessChecker: b.allowOwner,
 		},
 		{
 			key: optionNoSubject,
@@ -67,6 +80,7 @@ var commands = commandList{
 				optionNoSubject,
 			),
 			sanitizer:     utils.SanitizeBoolString,
+			accessChecker: b.allowOwner,
 		},
 		{
 			key: optionNoHTML,
@@ -75,6 +89,7 @@ var commands = commandList{
 				optionNoHTML,
 			),
 			sanitizer:     utils.SanitizeBoolString,
+			accessChecker: b.allowOwner,
 		},
 		{
 			key: optionNoThreads,
@@ -83,6 +98,7 @@ var commands = commandList{
 				optionNoThreads,
 			),
 			sanitizer:     utils.SanitizeBoolString,
+			accessChecker: b.allowOwner,
 		},
 		{
 			key: optionNoFiles,
@@ -91,11 +107,14 @@ var commands = commandList{
 				optionNoFiles,
 			),
 			sanitizer:     utils.SanitizeBoolString,
+			accessChecker: b.allowOwner,
 		},
 	}
+}
 
 func (b *Bot) handleCommand(ctx context.Context, evt *event.Event, commandSlice []string) {
-	if cmd := commands.get(commandSlice[0]); cmd == nil {
+	cmd := b.commands.get(commandSlice[0])
+	if cmd == nil {
 		return
 	}
 
@@ -104,11 +123,21 @@ func (b *Bot) handleCommand(ctx context.Context, evt *event.Event, commandSlice
 		return
 	}
 
+	allowed, err := cmd.accessChecker(evt.Sender, evt.RoomID)
+	if err != nil {
+		b.Error(ctx, evt.RoomID, err.Error())
+		return
+	}
+	if !allowed {
+		b.Notice(ctx, evt.RoomID, "not allowed to do that, kupo")
+		return
+	}
+
 	switch commandSlice[0] {
-	case "help":
+	case commandHelp:
 		b.sendHelp(ctx, evt.RoomID)
-	case "stop":
+	case commandStop:
-		b.runStop(ctx, true)
+		b.runStop(ctx)
 	default:
 		b.handleOption(ctx, commandSlice)
 	}
@@ -157,7 +186,7 @@ func (b *Bot) sendHelp(ctx context.Context, roomID id.RoomID) {
 
 	var msg strings.Builder
 	msg.WriteString("The following commands are supported:\n\n")
-	for _, cmd := range commands {
+	for _, cmd := range b.commands {
 		if cmd.key == "" {
 			msg.WriteString("\n---\n")
 			continue
@@ -192,7 +221,7 @@ func (b *Bot) sendHelp(ctx context.Context, roomID id.RoomID) {
 	b.Notice(ctx, roomID, msg.String())
 }
 
-func (b *Bot) runStop(ctx context.Context, checkAllowed bool) {
+func (b *Bot) runStop(ctx context.Context) {
 	evt := eventFromContext(ctx)
 	cfg, err := b.getSettings(evt.RoomID)
 	if err != nil {
@@ -200,11 +229,6 @@ func (b *Bot) runStop(ctx context.Context, checkAllowed bool) {
 		return
 	}
 
-	if checkAllowed && !b.Allowed(evt.Sender, cfg) {
-		b.Notice(ctx, evt.RoomID, "you don't have permission to do that")
-		return
-	}
-
 	mailbox := cfg.Get(optionMailbox)
 	if mailbox == "" {
 		b.Notice(ctx, evt.RoomID, "that room is not configured yet")
@@ -252,7 +276,7 @@ func (b *Bot) getOption(ctx context.Context, name string) {
 }
 
 func (b *Bot) setOption(ctx context.Context, name, value string) {
-	cmd := commands.get(name)
+	cmd := b.commands.get(name)
 	if cmd != nil {
 		value = cmd.sanitizer(value)
 	}
@@ -272,11 +296,6 @@ func (b *Bot) setOption(ctx context.Context, name, value string) {
 		return
 	}
 
-	if !b.Allowed(evt.Sender, cfg) {
-		b.Notice(ctx, evt.RoomID, "you don't have permission to do that, kupo")
-		return
-	}
-
 	old := cfg.Get(name)
 	cfg.Set(name, value)
 

bot/settings.go

@@ -98,21 +98,3 @@ func (b *Bot) getSettings(roomID id.RoomID) (settings, error) {
 func (b *Bot) setSettings(roomID id.RoomID, cfg settings) error {
 	return utils.UnwrapError(b.lp.GetClient().SetRoomAccountData(roomID, settingskey, cfg))
 }
-
-// Allowed checks if change is allowed
-func (b *Bot) Allowed(userID id.UserID, cfg settings) bool {
-	if !utils.Match(userID.String(), b.allowedUsers) {
-		return false
-	}
-
-	if b.noowner {
-		return true
-	}
-
-	owner := cfg.Owner()
-	if owner == "" {
-		return true
-	}
-
-	return owner == userID.String()
-}

bot/sync.go

@@ -94,7 +94,7 @@ func (b *Bot) onLeave(ctx context.Context) {
 	count := len(members)
 	if count == 1 && members[0] == b.lp.GetClient().UserID {
 		b.log.Info("no more users left in the %s room", evt.RoomID)
-		b.runStop(ctx, false)
+		b.runStop(ctx)
 		_, err := b.lp.GetClient().LeaveRoom(evt.RoomID)
 		if err != nil {
 			b.Error(ctx, evt.RoomID, "cannot leave empty room: %v", err)