s/whitelist/allowlist/ (user-facing); move helper functions outside the handler function

host.go

@@ -699,19 +699,67 @@ func (h *Host) InitCommands(c *chat.Commands) {
 		},
 	})
 
+	forConnectedUsers := func(cmd func(*chat.Member, ssh.PublicKey) error) error {
+		return h.Members.Each(func(key string, item set.Item) error {
+			v := item.Value()
+			if v == nil { // expired between Each and here
+				return nil
+			}
+			user := v.(*chat.Member)
+			pk := user.Identifier.(*Identity).PublicKey()
+			return cmd(user, pk)
+		})
+	}
+
+	forPubkeyUser := func(args []string, cmd func(ssh.PublicKey)) (errors []string) {
+		invalidUsers := []string{}
+		invalidKeys := []string{}
+		noKeyUsers := []string{}
+		var keyType string
+		for _, v := range args {
+			switch {
+			case keyType != "":
+				pk, _, _, _, err := ssh.ParseAuthorizedKey([]byte(keyType + " " + v))
+				if err == nil {
+					cmd(pk)
+				} else {
+					invalidKeys = append(invalidKeys, keyType+" "+v)
+				}
+				keyType = ""
+			case strings.HasPrefix(v, "ssh-"):
+				keyType = v
+			default:
+				user, ok := h.GetUser(v)
+				if ok {
+					pk := user.Identifier.(*Identity).PublicKey()
+					if pk == nil {
+						noKeyUsers = append(noKeyUsers, user.Identifier.Name())
+					} else {
+						cmd(pk)
+					}
+				} else {
+					invalidUsers = append(invalidUsers, v)
+				}
+			}
+		}
+		if len(noKeyUsers) != 0 {
+			errors = append(errors, fmt.Sprintf("users without a public key: %v", noKeyUsers))
+		}
+		if len(invalidUsers) != 0 {
+			errors = append(errors, fmt.Sprintf("invalid users: %v", invalidUsers))
+		}
+		if len(invalidKeys) != 0 {
+			errors = append(errors, fmt.Sprintf("invalid keys: %v", invalidKeys))
+		}
+		return
+	}
+
 	c.Add(chat.Command{
 		// TODO: default for reload
-		// TODO: reverify: what about passphrases?
-		//	- make this a different command (why? a passphrase can't change)
-		//	- who cares, kick them? -- after all, they can just reconnect
-		//	- store a flag in users that authenticated via passphrase and skip here (much more complicated)
-		//  - in which cases does this situation actually happen?
-		// TODO: "print" command with a format for saving to the whitelist file?
-		//   -> hard because the whitelist set only saves fingerprints
 		Op:         true,
-		Prefix:     "/whitelist",
+		Prefix:     "/allowlist",
 		PrefixHelp: "COMMAND [ARGS...]",
-		Help:       "Manipulate the whitelist or whitelist state. See /whitelist help for subcommands",
+		Help:       "Modify the allowlist or allowlist state. See /allowlist help for subcommands",
 		Handler: func(room *chat.Room, msg message.CommandMsg) error {
 			if !room.IsOp(msg.From()) {
 				return errors.New("must be op")
@@ -728,82 +776,32 @@ func (h *Host) InitCommands(c *chat.Commands) {
 				replyLines = append(replyLines, fmt.Sprintf(content, formatting...))
 			}
 
-			forConnectedUsers := func(cmd func(*chat.Member, ssh.PublicKey) error) error {
-				return h.Members.Each(func(key string, item set.Item) error {
-					v := item.Value()
-					if v == nil { // expired between Each and here
-						return nil
-					}
-					user := v.(*chat.Member)
-					pk := user.Identifier.(*Identity).PublicKey()
-					return cmd(user, pk)
-				})
-			}
-
-			forPubkeyUser := func(cmd func(ssh.PublicKey)) {
-				invalidUsers := []string{}
-				invalidKeys := []string{}
-				noKeyUsers := []string{}
-				var keyType string
-				for _, v := range args[1:] {
-					switch {
-					case keyType != "":
-						pk, _, _, _, err := ssh.ParseAuthorizedKey([]byte(keyType + " " + v))
-						if err == nil {
-							cmd(pk)
-						} else {
-							invalidKeys = append(invalidKeys, keyType+" "+v)
-						}
-						keyType = ""
-					case strings.HasPrefix(v, "ssh-"):
-						keyType = v
-					default:
-						user, ok := h.GetUser(v)
-						if ok {
-							pk := user.Identifier.(*Identity).PublicKey()
-							if pk == nil {
-								noKeyUsers = append(noKeyUsers, user.Identifier.Name())
-							} else {
-								cmd(pk)
-							}
-						} else {
-							invalidUsers = append(invalidUsers, v)
-						}
-					}
-				}
-				if len(noKeyUsers) != 0 {
-					sendMsg("users without a public key: %v", noKeyUsers)
-				}
-				if len(invalidUsers) != 0 {
-					sendMsg("invalid users: %v", invalidUsers)
-				}
-				if len(invalidKeys) != 0 {
-					sendMsg("invalid keys: %v", invalidKeys)
-				}
-			}
-
 			switch args[0] {
 			case "help":
-				sendMsg("Usage: /whitelist help | on | off | add {PUBKEY|USER}... | remove {PUBKEY|USER}... | import [AGE] | reload {keep|flush} | reverify | status")
+				sendMsg("Usage: /allowlist help | on | off | add {PUBKEY|USER}... | remove {PUBKEY|USER}... | import [AGE] | reload {keep|flush} | reverify | status")
 				sendMsg("help: this help message")
-				sendMsg("on, off: set whitelist mode (applies to new connections)")
+				sendMsg("on, off: set allowlist mode (applies to new connections)")
-				sendMsg("add, remove: add or remove keys from the whitelist")
+				sendMsg("add, remove: add or remove keys from the allowlist")
-				sendMsg("import: add all keys of users connected since AGE (default 0) ago to the whitelist")
+				sendMsg("import: add all keys of users connected since AGE (default 0) ago to the allowlist")
-				sendMsg("reload: re-read the whitelist file and keep or discard entries in the current whitelist but not in the file")
+				sendMsg("reload: re-read the allowlist file and keep or discard entries in the current allowlist but not in the file")
-				sendMsg("reverify: kick all users not in the whitelist if whitelisting is enabled")
+				sendMsg("reverify: kick all users not in the allowlist if allowlisting is enabled")
 				sendMsg("status: show status information")
 			case "on":
 				h.auth.SetWhitelistMode(true)
 			case "off":
 				h.auth.SetWhitelistMode(false)
 			case "add":
-				forPubkeyUser(func(pk ssh.PublicKey) { h.auth.Whitelist(pk, 0) })
+				for _, errLine := range forPubkeyUser(args[1:], func(pk ssh.PublicKey) { h.auth.Whitelist(pk, 0) }) {
+					sendMsg(errLine)
+				}
 			case "remove":
-				forPubkeyUser(func(pk ssh.PublicKey) { h.auth.Whitelist(pk, 1) })
+				for _, errLine := range forPubkeyUser(args[1:], func(pk ssh.PublicKey) { h.auth.Whitelist(pk, 1) }) {
+					sendMsg(errLine)
+				}
 			case "import":
 				var since time.Duration
-				var err error
 				if len(args) > 1 {
+					var err error
 					since, err = time.ParseDuration(args[1])
 					if err != nil {
 						return err
@@ -837,7 +835,7 @@ func (h *Host) InitCommands(c *chat.Commands) {
 				}
 			case "reverify":
 				if !h.auth.WhitelistMode() {
-					sendMsg("whitelist is disabled, so nobody will be kicked")
+					sendMsg("allowlist is disabled, so nobody will be kicked")
 					break
 				}
 				forConnectedUsers(func(user *chat.Member, pk ssh.PublicKey) error {
@@ -848,9 +846,9 @@ func (h *Host) InitCommands(c *chat.Commands) {
 				})
 			case "status":
 				if h.auth.WhitelistMode() {
-					sendMsg("The whitelist is currently enabled.")
+					sendMsg("The allowlist is currently enabled.")
 				} else {
-					sendMsg("The whitelist is currently disabled.")
+					sendMsg("The allowlist is currently disabled.")
 				}
 				whitelistedUsers := []string{}
 				whitelistedKeys := []string{}
@@ -870,10 +868,10 @@ func (h *Host) InitCommands(c *chat.Commands) {
 					return nil
 				})
 				if len(whitelistedUsers) != 0 {
-					sendMsg("The following connected users are on the whitelist: %v", whitelistedUsers)
+					sendMsg("The following connected users are on the allowlist: %v", whitelistedUsers)
 				}
 				if len(whitelistedKeys) != 0 {
-					sendMsg("The following keys of not connected users are on the whitelist: %v", whitelistedKeys)
+					sendMsg("The following keys of not connected users are on the allowlist: %v", whitelistedKeys)
 				}
 			default:
 				return errors.New("invalid subcommand: " + args[0])

host_test.go

@@ -201,7 +201,7 @@ func TestHostWhitelist(t *testing.T) {
 	}
 }
 
-func TestHostWhitelistCommand(t *testing.T) {
+func TestHostAllowlistCommand(t *testing.T) {
 	s, host := getHost(t, NewAuth())
 	defer s.Close()
 	go host.Serve()
@@ -233,10 +233,10 @@ func TestHostWhitelistCommand(t *testing.T) {
 			host.HandleMsg(message.ParseInput(fmt.Sprintf(cmd, formatting...), m.User))
 		}
 
-		sendCmd("/whitelist")
+		sendCmd("/allowlist")
 		assertLineEq("Err: must be op\r")
 		m.IsOp = true
-		sendCmd("/whitelist")
+		sendCmd("/allowlist")
 		for _, expected := range [...]string{"Usage", "help", "on, off", "add, remove", "import", "reload", "reverify", "status"} {
 			if !scanner.Scan() {
 				t.Error("no line available")
@@ -246,13 +246,13 @@ func TestHostWhitelistCommand(t *testing.T) {
 			}
 		}
 
-		sendCmd("/whitelist on")
+		sendCmd("/allowlist on")
 		if !host.auth.WhitelistMode() {
-			t.Error("whitelist not enabled after /whitelist on")
+			t.Error("allowlist not enabled after /allowlist on")
 		}
-		sendCmd("/whitelist off")
+		sendCmd("/allowlist off")
 		if host.auth.WhitelistMode() {
-			t.Error("whitelist not disabled after /whitelist off")
+			t.Error("allowlist not disabled after /allowlist off")
 		}
 
 		// TODO: can we pass a public key when connecting?
@@ -263,49 +263,49 @@ func TestHostWhitelistCommand(t *testing.T) {
 		testKey2 := "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAINDnlvlhBf4Jx7RlqTO6C5iOUhsBk2CHOpwPgPUbo8vb"
 		testKey2FP := "SHA256:tMBXmUCPMxbSNj1pzQlGR+N2RiAIvcnqT18vX0r2rrM="
 
-		sendCmd("/whitelist add ssh-invalid blah ssh-rsa wrongAsWell invalid foo %s %s", testKey1, testKey2)
+		sendCmd("/allowlist add ssh-invalid blah ssh-rsa wrongAsWell invalid foo %s %s", testKey1, testKey2)
 		assertLineEq("users without a public key: [foo]\r")
 		assertLineEq("invalid users: [invalid]\r")
 		assertLineEq("invalid keys: [ssh-invalid blah ssh-rsa wrongAsWell]\r")
 		if !host.auth.whitelist.In(testKey1FP) || !host.auth.whitelist.In(testKey2FP) {
-			t.Error("failed to add keys to whitelist")
+			t.Error("failed to add keys to allowlist")
 		}
-		sendCmd("/whitelist remove invalid %s", testKey1)
+		sendCmd("/allowlist remove invalid %s", testKey1)
 		assertLineEq("invalid users: [invalid]\r")
 		if host.auth.whitelist.In(testKey1FP) {
-			t.Error("failed to remove key from whitelist")
+			t.Error("failed to remove key from allowlist")
 		}
 		if !host.auth.whitelist.In(testKey2FP) {
 			t.Error("removed wrong key")
 		}
 
 		// TODO: to test the AGE arg, we need another connection and possibly a sleep
-		sendCmd("/whitelist import")
+		sendCmd("/allowlist import")
 		assertLineEq("users without a public key: [foo]\r")
 
 		// TODO: test reload with files?
-		sendCmd("/whitelist reload keep")
+		sendCmd("/allowlist reload keep")
 		if !host.auth.whitelist.In(testKey2FP) {
-			t.Error("cleared whitelist to be kept")
+			t.Error("cleared allowlist to be kept")
 		}
-		sendCmd("/whitelist reload flush")
+		sendCmd("/allowlist reload flush")
 		if host.auth.whitelist.In(testKey2FP) {
-			t.Error("kept whitelist to be cleared")
+			t.Error("kept allowlist to be cleared")
 		}
-		sendCmd("/whitelist reload thisIsWrong")
+		sendCmd("/allowlist reload thisIsWrong")
 		assertLineEq("Err: must specify whether to keep or flush current entries\r")
-		sendCmd("/whitelist reload")
+		sendCmd("/allowlist reload")
 		assertLineEq("Err: must specify whether to keep or flush current entries\r")
 
-		sendCmd("/whitelist reverify")
+		sendCmd("/allowlist reverify")
-		assertLineEq("whitelist is disabled, so nobody will be kicked\r")
+		assertLineEq("allowlist is disabled, so nobody will be kicked\r")
 
-		sendCmd("/whitelist add " + testKey1)
+		sendCmd("/allowlist add " + testKey1)
-		sendCmd("/whitelist status")
+		sendCmd("/allowlist status")
-		assertLineEq("The whitelist is currently disabled.\r")
+		assertLineEq("The allowlist is currently disabled.\r")
-		assertLineEq(fmt.Sprintf("The following keys of not connected users are on the whitelist: [%s]\r", testKey1FP))
+		assertLineEq(fmt.Sprintf("The following keys of not connected users are on the allowlist: [%s]\r", testKey1FP))
 
-		sendCmd("/whitelist invalidSubcommand")
+		sendCmd("/allowlist invalidSubcommand")
 		assertLineEq("Err: invalid subcommand: invalidSubcommand\r")
 		return nil
 	})