fix various bugs

irc/accounts.go

@@ -943,6 +943,10 @@ func (am *AccountManager) checkPassphrase(accountName, passphrase string) (accou
 }
 
 func (am *AccountManager) AuthenticateByPassphrase(client *Client, accountName string, passphrase string) (err error) {
+	// XXX check this now, so we don't allow a redundant login for an always-on client
+	// even for a brief period. the other potential source of nick-account conflicts
+	// is from force-nick-equals-account, but those will be caught later by
+	// fixupNickEqualsAccount and if there is a conflict, they will be logged out.
 	if client.registered {
 		if clientAlready := am.server.clients.Get(accountName); clientAlready != nil && clientAlready.AlwaysOn() {
 			return errNickAccountMismatch

irc/errors.go

@@ -41,8 +41,7 @@ var (
 	errNicknameInvalid                = errors.New("invalid nickname")
 	errNicknameInUse                  = errors.New("nickname in use")
 	errNicknameReserved               = errors.New("nickname is reserved")
-	errCantChangeNick                 = errors.New(`You must use your account name as your nickname`)
+	errNickAccountMismatch            = errors.New(`Your nickname must match your account name; try logging out and logging back in with SASL`)
-	errNickAccountMismatch            = errors.New(`Your nickname must match your account name`)
 	errNoExistingBan                  = errors.New("Ban does not exist")
 	errNoSuchChannel                  = errors.New(`No such channel`)
 	errChannelPurged                  = errors.New(`This channel was purged by the server operators and cannot be used`)

irc/handlers.go

@@ -237,6 +237,8 @@ func authPlainHandler(server *Server, client *Client, mechanism string, value []
 		msg := authErrorToMessage(server, err)
 		rb.Add(nil, server.name, ERR_SASLFAIL, client.Nick(), fmt.Sprintf("%s: %s", client.t("SASL authentication failed"), client.t(msg)))
 		return false
+	} else if !fixupNickEqualsAccount(client, rb, server.Config()) {
+		return false
 	}
 
 	sendSuccessfulAccountAuth(client, rb, false, true)
@@ -245,7 +247,7 @@ func authPlainHandler(server *Server, client *Client, mechanism string, value []
 
 func authErrorToMessage(server *Server, err error) (msg string) {
 	switch err {
-	case errAccountDoesNotExist, errAccountUnverified, errAccountInvalidCredentials, errAuthzidAuthcidMismatch:
+	case errAccountDoesNotExist, errAccountUnverified, errAccountInvalidCredentials, errAuthzidAuthcidMismatch, errNickAccountMismatch:
 		return err.Error()
 	default:
 		// don't expose arbitrary error messages to the user
@@ -280,6 +282,8 @@ func authExternalHandler(server *Server, client *Client, mechanism string, value
 		msg := authErrorToMessage(server, err)
 		rb.Add(nil, server.name, ERR_SASLFAIL, client.nick, fmt.Sprintf("%s: %s", client.t("SASL authentication failed"), client.t(msg)))
 		return false
+	} else if !fixupNickEqualsAccount(client, rb, server.Config()) {
+		return false
 	}
 
 	sendSuccessfulAccountAuth(client, rb, false, true)

irc/nickname.go

@@ -37,9 +37,9 @@ func performNickChange(server *Server, client *Client, target *Client, session *
 
 	assignedNickname, err := client.server.clients.SetNick(target, session, nickname)
 	if err == errNicknameInUse {
-		rb.Add(nil, server.name, ERR_NICKNAMEINUSE, currentNick, nickname, client.t("Nickname is already in use"))
+		rb.Add(nil, server.name, ERR_NICKNAMEINUSE, currentNick, utils.SafeErrorParam(nickname), client.t("Nickname is already in use"))
 	} else if err == errNicknameReserved {
-		rb.Add(nil, server.name, ERR_NICKNAMEINUSE, currentNick, nickname, client.t("Nickname is reserved by a different account"))
+		rb.Add(nil, server.name, ERR_NICKNAMEINUSE, currentNick, utils.SafeErrorParam(nickname), client.t("Nickname is reserved by a different account"))
 	} else if err == errNicknameInvalid {
 		rb.Add(nil, server.name, ERR_ERRONEUSNICKNAME, currentNick, utils.SafeErrorParam(nickname), client.t("Erroneous nickname"))
 	} else if err == errNickAccountMismatch {
@@ -108,3 +108,23 @@ func (server *Server) RandomlyRename(client *Client) {
 	// technically performNickChange can fail to change the nick,
 	// but if they're still delinquent, the timer will get them later
 }
+
+// if force-nick-equals-account is set, account name and nickname must be equal,
+// so we need to re-NICK automatically on every login event (IDENTIFY,
+// VERIFY, and a REGISTER that auto-verifies). if we can't get the nick
+// then we log them out (they will be able to reattach with SASL)
+func fixupNickEqualsAccount(client *Client, rb *ResponseBuffer, config *Config) (success bool) {
+	if !config.Accounts.NickReservation.ForceNickEqualsAccount {
+		return true
+	}
+	if !client.registered {
+		return true
+	}
+	// don't need to supply a nickname, SetNick will use the account name
+	if !performNickChange(client.server, client, client, rb.session, "", rb) {
+		client.server.accounts.Logout(client)
+		nsNotice(rb, client.t("A client is already using that account; try logging out and logging back in with SASL"))
+		return false
+	}
+	return true
+}

irc/nickserv.go

@@ -610,23 +610,6 @@ func nsLoginThrottleCheck(client *Client, rb *ResponseBuffer) (success bool) {
 	return true
 }
 
-// if force-nick-equals-account is set, account name and nickname must be equal,
-// so we need to re-NICK automatically on every login event (IDENTIFY,
-// VERIFY, and a REGISTER that auto-verifies). if we can't get the nick
-// then we log them out (they will be able to reattach with SASL)
-func nsFixNickname(client *Client, rb *ResponseBuffer, config *Config) (success bool) {
-	if !config.Accounts.NickReservation.ForceNickEqualsAccount {
-		return true
-	}
-	// don't need to supply a nickname, SetNick will use the account name
-	if !performNickChange(client.server, client, client, rb.session, "", rb) {
-		client.server.accounts.Logout(client)
-		nsNotice(rb, client.t("A client is already using that account; try logging out and logging back in with SASL"))
-		return false
-	}
-	return true
-}
-
 func nsIdentifyHandler(server *Server, client *Client, command string, params []string, rb *ResponseBuffer) {
 	if client.LoggedIntoAccount() {
 		nsNotice(rb, client.t("You're already logged into an account"))
@@ -666,17 +649,19 @@ func nsIdentifyHandler(server *Server, client *Client, command string, params []
 		loginSuccessful = (err == nil)
 	}
 
+	nickFixupFailed := false
 	if loginSuccessful {
-		if !nsFixNickname(client, rb, server.Config()) {
+		if !fixupNickEqualsAccount(client, rb, server.Config()) {
 			loginSuccessful = false
-			err = errNickAccountMismatch
+			// fixupNickEqualsAccount sends its own error message, don't send another
+			nickFixupFailed = true
 		}
 	}
 
 	if loginSuccessful {
 		sendSuccessfulAccountAuth(client, rb, true, true)
-	} else if err != errNickAccountMismatch {
+	} else if !nickFixupFailed {
-		nsNotice(rb, client.t("Could not login with your TLS certificate or supplied username/password"))
+		nsNotice(rb, fmt.Sprintf(client.t("Authentication failed: %s"), authErrorToMessage(server, err)))
 	}
 }
 
@@ -786,7 +771,7 @@ func nsRegisterHandler(server *Server, client *Client, command string, params []
 	if err == nil {
 		if callbackNamespace == "*" {
 			err = server.accounts.Verify(client, account, "")
-			if err == nil && nsFixNickname(client, rb, config) {
+			if err == nil && fixupNickEqualsAccount(client, rb, config) {
 				sendSuccessfulRegResponse(client, rb, true)
 			}
 		} else {
@@ -892,7 +877,7 @@ func nsVerifyHandler(server *Server, client *Client, command string, params []st
 		return
 	}
 
-	if nsFixNickname(client, rb, server.Config()) {
+	if fixupNickEqualsAccount(client, rb, server.Config()) {
 		sendSuccessfulRegResponse(client, rb, true)
 	}
 }