From 0b644065b79c63e63776179a955d6f7a84c85b42 Mon Sep 17 00:00:00 2001
From: Daniel Oaks <daniel@danieloaks.net>
Date: Mon, 8 Apr 2019 11:36:48 +1000
Subject: [PATCH] Review and spec updates

---
 irc/config.go   |  2 ++
 irc/handlers.go | 27 +++++++++++++++------------
 irc/help.go     |  3 ++-
 irc/nickserv.go |  3 ++-
 4 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/irc/config.go b/irc/config.go
index 524c73c4..7a655dde 100644
--- a/irc/config.go
+++ b/irc/config.go
@@ -13,6 +13,7 @@ import (
 	"net"
 	"os"
 	"regexp"
+	"sort"
 	"strings"
 	"time"
 
@@ -609,6 +610,7 @@ func LoadConfig(filename string) (config *Config, err error) {
 			config.Accounts.Registration.EnabledCallbacks[i] = "*"
 		}
 	}
+	sort.Strings(config.Accounts.Registration.EnabledCallbacks)
 
 	config.Accounts.RequireSasl.exemptedNets, err = utils.ParseNetList(config.Accounts.RequireSasl.Exempted)
 	if err != nil {
diff --git a/irc/handlers.go b/irc/handlers.go
index 9bfcb1fb..399f54fa 100644
--- a/irc/handlers.go
+++ b/irc/handlers.go
@@ -40,18 +40,17 @@ func accHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Respo
 
 		rb.Add(nil, server.name, "ACC", "LS", "SUBCOMMANDS", "LS REGISTER VERIFY")
 
-		var enabledCallbacks []string
-		for _, name := range config.Registration.EnabledCallbacks {
-			enabledCallbacks = append(enabledCallbacks, name)
-		}
-		sort.Strings(enabledCallbacks)
-		rb.Add(nil, server.name, "ACC", "LS", "CALLBACKS", strings.Join(enabledCallbacks, " "))
+		// this list is sorted by the config loader, yay
+		rb.Add(nil, server.name, "ACC", "LS", "CALLBACKS", strings.Join(config.Registration.EnabledCallbacks, " "))
 
 		rb.Add(nil, server.name, "ACC", "LS", "CREDTYPES", "passphrase certfp")
 
+		flags := []string{"nospaces"}
 		if config.NickReservation.Enabled {
-			rb.Add(nil, server.name, "ACC", "LS", "FLAGS", "regnick")
+			flags = append(flags, "regnick")
 		}
+		sort.Strings(flags)
+		rb.Add(nil, server.name, "ACC", "LS", "FLAGS", strings.Join(flags, " "))
 		return false
 	}
 
@@ -113,7 +112,7 @@ func accRegisterHandler(server *Server, client *Client, msg ircmsg.IrcMessage, r
 		return false
 	}
 
-	account := strings.TrimSpace(msg.Params[1])
+	account := msg.Params[1]
 
 	// check for account name of *
 	if account == "*" {
@@ -166,7 +165,7 @@ func accRegisterHandler(server *Server, client *Client, msg ircmsg.IrcMessage, r
 		}
 	}
 	if credentialType == "certfp" && client.certfp == "" {
-		rb.Add(nil, server.name, "FAIL", "ACC", "REG_INVALID_CRED_TYPE", account, credentialType, client.t("You are not using a TLS certificate"))
+		rb.Add(nil, server.name, "FAIL", "ACC", "REG_INVALID_CREDENTIAL", account, client.t("You must connect with a TLS client certificate to use certfp"))
 		return false
 	}
 
@@ -190,8 +189,8 @@ func accRegisterHandler(server *Server, client *Client, msg ircmsg.IrcMessage, r
 
 	err = server.accounts.Register(client, account, callbackNamespace, callbackValue, passphrase, certfp)
 	if err != nil {
-		msg := registrationErrorToMessageAndCode(err)
-		rb.Add(nil, server.name, "FAIL", "ACC", "REG_UNSPECIFIED_ERROR", account, client.t(msg))
+		msg, code := registrationErrorToMessageAndCode(err)
+		rb.Add(nil, server.name, "FAIL", "ACC", code, account, client.t(msg))
 		return false
 	}
 
@@ -211,11 +210,15 @@ func accRegisterHandler(server *Server, client *Client, msg ircmsg.IrcMessage, r
 	return false
 }
 
-func registrationErrorToMessageAndCode(err error) (message string) {
+func registrationErrorToMessageAndCode(err error) (message, code string) {
 	// default responses: let's be risk-averse about displaying internal errors
 	// to the clients, especially for something as sensitive as accounts
+	code = "REG_UNSPECIFIED_ERROR"
 	message = `Could not register`
 	switch err {
+	case errAccountBadPassphrase:
+		code = "REG_INVALID_CREDENTIAL"
+		message = err.Error()
 	case errAccountAlreadyRegistered, errAccountAlreadyVerified:
 		message = err.Error()
 	case errAccountCreation, errAccountMustHoldNick, errAccountBadPassphrase, errCertfpAlreadyExists, errFeatureDisabled:
diff --git a/irc/help.go b/irc/help.go
index 5bbffa48..38bb62d5 100644
--- a/irc/help.go
+++ b/irc/help.go
@@ -97,7 +97,8 @@ For instance, this would set the kill, oper, account and xline snomasks on dan:
 var Help = map[string]HelpEntry{
 	// Commands
 	"acc": {
-		text: `ACC REGISTER <accountname> [callback_namespace:]<callback> [cred_type] :<credential>
+		text: `ACC LS
+ACC REGISTER <accountname> [callback_namespace:]<callback> [cred_type] :<credential>
 ACC VERIFY <accountname> <auth_code>
 
 Used in account registration. See the relevant specs for more info:
diff --git a/irc/nickserv.go b/irc/nickserv.go
index 2a991836..bc5eaf3c 100644
--- a/irc/nickserv.go
+++ b/irc/nickserv.go
@@ -406,8 +406,9 @@ func nsRegisterHandler(server *Server, client *Client, command string, params []
 	}
 
 	// details could not be stored and relevant numerics have been dispatched, abort
+	message, _ := registrationErrorToMessageAndCode(err)
 	if err != nil {
-		nsNotice(rb, client.t(registrationErrorToMessageAndCode(err)))
+		nsNotice(rb, client.t(message))
 		return
 	}
 }