From 46572b871fb994b62adbebb22406f3055bba013b Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Wed, 7 Jul 2021 06:35:30 -0400 Subject: [PATCH] expose a user-visible error if direct email sending fails See #1659 --- irc/accounts.go | 26 +++++++++++++++++++++++++- irc/email/email.go | 9 +++++++-- irc/errors.go | 1 - irc/handlers.go | 16 ++++++++++------ irc/nickserv.go | 2 +- 5 files changed, 43 insertions(+), 11 deletions(-) diff --git a/irc/accounts.go b/irc/accounts.go index 473b21fa..8bf714f9 100644 --- a/irc/accounts.go +++ b/irc/accounts.go @@ -15,6 +15,8 @@ import ( "time" "unicode" + "github.com/ergochat/irc-go/ircutils" + "github.com/ergochat/ergo/irc/connection_limits" "github.com/ergochat/ergo/irc/email" "github.com/ergochat/ergo/irc/migrations" @@ -460,7 +462,7 @@ func (am *AccountManager) Register(client *Client, account string, callbackNames code, err := am.dispatchCallback(client, account, callbackNamespace, callbackValue) if err != nil { am.Unregister(casefoldedAccount, true) - return errCallbackFailed + return ®istrationCallbackError{underlying: err} } else { return am.server.store.Update(func(tx *buntdb.Tx) error { _, _, err = tx.Set(verificationCodeKey, code, setOptions) @@ -469,6 +471,28 @@ func (am *AccountManager) Register(client *Client, account string, callbackNames } } +type registrationCallbackError struct { + underlying error +} + +func (r *registrationCallbackError) Error() string { + return `Account verification could not be sent` +} + +func registrationCallbackErrorText(config *Config, client *Client, err error) string { + if callbackErr, ok := err.(*registrationCallbackError); ok { + // only expose a user-visible error if we are doing direct sending + if config.Accounts.Registration.EmailVerification.DirectSendingEnabled() { + errorText := ircutils.SanitizeText(callbackErr.underlying.Error(), 350) + return fmt.Sprintf(client.t("Could not dispatch registration e-mail: %s"), errorText) + } else { + return client.t("Could not dispatch registration e-mail") + } + } else { + return "" + } +} + // validatePassphrase checks whether a passphrase is allowed by our rules func validatePassphrase(passphrase string) error { // sanity check the length diff --git a/irc/email/email.go b/irc/email/email.go index dad129c2..11139fd9 100644 --- a/irc/email/email.go +++ b/irc/email/email.go @@ -15,7 +15,7 @@ import ( var ( ErrBlacklistedAddress = errors.New("Email address is blacklisted") - ErrInvalidAddress = errors.New("Email address is blacklisted") + ErrInvalidAddress = errors.New("Email address is invalid") ErrNoMXRecord = errors.New("Couldn't resolve MX record") ) @@ -73,6 +73,11 @@ func (config *MailtoConfig) Postprocess(heloDomain string) (err error) { return config.DKIM.Postprocess() } +// are we sending email directly, as opposed to deferring to an MTA? +func (config *MailtoConfig) DirectSendingEnabled() bool { + return config.MTAReal.Server == "" +} + // get the preferred MX record hostname, "" on error func lookupMX(domain string) (server string) { var minPref uint16 @@ -104,7 +109,7 @@ func SendMail(config MailtoConfig, recipient string, msg []byte) (err error) { var addr string var auth smtp.Auth - if config.MTAReal.Server != "" { + if !config.DirectSendingEnabled() { addr = fmt.Sprintf("%s:%d", config.MTAReal.Server, config.MTAReal.Port) if config.MTAReal.Username != "" && config.MTAReal.Password != "" { auth = smtp.PlainAuth("", config.MTAReal.Username, config.MTAReal.Password, config.MTAReal.Server) diff --git a/irc/errors.go b/irc/errors.go index 30eb3e54..85ce343c 100644 --- a/irc/errors.go +++ b/irc/errors.go @@ -34,7 +34,6 @@ var ( errAccountUpdateFailed = errors.New(`Error while updating your account information`) errAccountMustHoldNick = errors.New(`You must hold that nickname in order to register it`) errAuthzidAuthcidMismatch = errors.New(`authcid and authzid must be the same`) - errCallbackFailed = errors.New("Account verification could not be sent") errCertfpAlreadyExists = errors.New(`An account already exists for your certificate fingerprint`) errChannelNotOwnedByAccount = errors.New("Channel not owned by the specified account") errChannelTransferNotOffered = errors.New(`You weren't offered ownership of that channel`) diff --git a/irc/handlers.go b/irc/handlers.go index bd15175d..15e9f0c2 100644 --- a/irc/handlers.go +++ b/irc/handlers.go @@ -63,14 +63,16 @@ func parseCallback(spec string, config *Config) (callbackNamespace string, callb return } -func registrationErrorToMessage(err error) (message string) { +func registrationErrorToMessage(config *Config, client *Client, err error) (message string) { + if emailError := registrationCallbackErrorText(config, client, err); emailError != "" { + return emailError + } + switch err { case errAccountAlreadyRegistered, errAccountAlreadyVerified, errAccountAlreadyUnregistered, errAccountAlreadyLoggedIn, errAccountCreation, errAccountMustHoldNick, errAccountBadPassphrase, errCertfpAlreadyExists, errFeatureDisabled, errAccountBadPassphrase: message = err.Error() case errLimitExceeded: message = `There have been too many registration attempts recently; try again later` - case errCallbackFailed: - message = `Could not dispatch verification email` default: // default response: let's be risk-averse about displaying internal errors // to the clients, especially for something as sensitive as accounts @@ -2548,10 +2550,12 @@ func registerHandler(server *Server, client *Client, msg ircmsg.Message, rb *Res rb.Add(nil, server.name, "FAIL", "REGISTER", "USERNAME_EXISTS", accountName, client.t("Username is already registered or otherwise unavailable")) case errAccountBadPassphrase: rb.Add(nil, server.name, "FAIL", "REGISTER", "INVALID_PASSWORD", accountName, client.t("Password was invalid")) - case errCallbackFailed: - rb.Add(nil, server.name, "FAIL", "REGISTER", "UNACCEPTABLE_EMAIL", accountName, client.t("Could not dispatch verification e-mail")) default: - rb.Add(nil, server.name, "FAIL", "REGISTER", "UNKNOWN_ERROR", accountName, client.t("Could not register")) + if emailError := registrationCallbackErrorText(config, client, err); emailError != "" { + rb.Add(nil, server.name, "FAIL", "REGISTER", "UNACCEPTABLE_EMAIL", accountName, emailError) + } else { + rb.Add(nil, server.name, "FAIL", "REGISTER", "UNKNOWN_ERROR", accountName, client.t("Could not register")) + } } return } diff --git a/irc/nickserv.go b/irc/nickserv.go index 364435ff..2cd62fb7 100644 --- a/irc/nickserv.go +++ b/irc/nickserv.go @@ -900,7 +900,7 @@ func nsRegisterHandler(service *ircService, server *Server, client *Client, comm } } else { // details could not be stored and relevant numerics have been dispatched, abort - message := registrationErrorToMessage(err) + message := registrationErrorToMessage(config, client, err) service.Notice(rb, client.t(message)) } }