From 9e9dee43acd77ea3574baded2b574050477ceaa0 Mon Sep 17 00:00:00 2001 From: James Elliott Date: Tue, 21 Apr 2020 14:59:38 +1000 Subject: [PATCH] [FEATURE] Notifier Startup Checks (#889) * implement SMTP notifier startup check * check dial, starttls, auth, mail from, rcpt to, reset, and quit * log the error on failure * implement mock * misc optimizations, adjustments, and refactoring * implement validate_skip config option * fix comments to end with period * fix suites that used smtp notifier without a smtp container * add docs * add file notifier startup check * move file mode into const.go * disable gosec linting on insecureskipverify since it's intended, warned, and discouraged * minor PR commentary adjustment * apply suggestions from code review Co-Authored-By: Amir Zarrinkafsh --- .github/probot.js | 9 +- cmd/authelia/main.go | 6 + config.template.yml | 9 +- docs/configuration/notifier/filesystem.md | 11 +- docs/configuration/notifier/index.md | 17 +++ docs/configuration/notifier/smtp.md | 11 +- internal/configuration/schema/notifier.go | 24 ++-- internal/configuration/validator/notifier.go | 3 + internal/mocks/mock_notifier.go | 17 ++- internal/notification/const.go | 3 + internal/notification/file_notifier.go | 37 +++++- internal/notification/notifier.go | 1 + internal/notification/smtp_notifier.go | 125 ++++++++++++------ internal/suites/DuoPush/configuration.yml | 8 +- .../suites/OneFactorOnly/configuration.yml | 7 +- 15 files changed, 205 insertions(+), 83 deletions(-) create mode 100644 internal/notification/const.go diff --git a/.github/probot.js b/.github/probot.js index 22782e305..140a6cc21 100644 --- a/.github/probot.js +++ b/.github/probot.js @@ -8,10 +8,10 @@ on('pull_request.opened') context => context.payload.pull_request.head.ref.slice(0, 11) !== 'dependabot/' ) - .comment(`# Artifacts + .comment(`## Artifacts These changes are published for testing on Buildkite and DockerHub. -## Docker Container +### Docker Container * \`docker pull authelia/authelia:{{ pull_request.head.ref }}\``) // PR commentary for third party based contributions @@ -21,10 +21,11 @@ on('pull_request.opened') context.payload.pull_request.head.label.slice(0, 9) !== 'authelia:' ) .comment(`Thanks for choosing to contribute. We lint all PR's with golangci-lint, autheliabot may add a review to your PR with some suggestions. + You are free to apply the changes if you're comfortable, alternatively you are welcome to ask a team member for advice. -# Artifacts +## Artifacts These changes once approved by a team member will be published for testing on Buildkite and DockerHub. -## Docker Container +### Docker Container * \`docker pull authelia/authelia:PR{{ pull_request.number }}\``) \ No newline at end of file diff --git a/cmd/authelia/main.go b/cmd/authelia/main.go index 364f69d70..0b02b5edd 100644 --- a/cmd/authelia/main.go +++ b/cmd/authelia/main.go @@ -89,6 +89,12 @@ func startServer() { } else { log.Fatalf("Unrecognized notifier") } + if !config.Notifier.DisableStartupCheck { + _, err := notifier.StartupCheck() + if err != nil { + log.Fatalf("Error during notifier startup check: %s", err) + } + } clock := utils.RealClock{} authorizer := authorization.NewAuthorizer(config.AccessControl) diff --git a/config.template.yml b/config.template.yml index ca5718360..6489dda85 100644 --- a/config.template.yml +++ b/config.template.yml @@ -350,8 +350,11 @@ storage: # # Notifications are sent to users when they require a password reset, a u2f # registration or a TOTP registration. -# Use only an available configuration: filesystem, gmail +# Use only an available configuration: filesystem, smtp. notifier: + # You can disable the notifier startup check by setting this to true. + disable_startup_check: false + # For testing purpose, notifications can be sent in a file ## filesystem: ## filename: /tmp/authelia/notification.txt @@ -377,9 +380,11 @@ notifier: # Subject configuration of the emails sent. # {title} is replaced by the text from the notifier subject: "[Authelia] {title}" + # This address is used during the startup check to verify the email configuration is correct. It's not important what it is except if your email server only allows local delivery. + ## startup_check_address: test@authelia.com + ## trusted_cert: "" ## disable_require_tls: false ## disable_verify_cert: false - ## trusted_cert: "" # Sending an email using a Gmail account is as simple as the next section. # You need to create an app password by following: https://support.google.com/accounts/answer/185833?hl=en diff --git a/docs/configuration/notifier/filesystem.md b/docs/configuration/notifier/filesystem.md index 38835eb9e..c3267c9e0 100644 --- a/docs/configuration/notifier/filesystem.md +++ b/docs/configuration/notifier/filesystem.md @@ -12,7 +12,16 @@ With this configuration, the message will be sent to a file. This option should only be used for testing purposes. ```yaml +# Configuration of the notification system. +# +# Notifications are sent to users when they require a password reset, a U2F +# registration or a TOTP registration. +# Use only an available configuration: filesystem, smtp. notifier: + # You can disable the notifier startup check by setting this to true. + disable_startup_check: false + + # For testing purpose, notifications can be sent in a file. filesystem: filename: /tmp/authelia/notification.txt -``` \ No newline at end of file +``` diff --git a/docs/configuration/notifier/index.md b/docs/configuration/notifier/index.md index 5d05b6cc7..353b05079 100644 --- a/docs/configuration/notifier/index.md +++ b/docs/configuration/notifier/index.md @@ -10,3 +10,20 @@ has_children: true **Authelia** sometimes needs to send messages to users in order to verify their identity. + +## Startup Check + +The notifier has a startup check which validates the specified provider +configuration is correct and will be able to send emails. This can be +disabled with the `disable_startup_check` option: + +```yaml +# Configuration of the notification system. +# +# Notifications are sent to users when they require a password reset, a u2f +# registration or a TOTP registration. +# Use only an available configuration: filesystem, smtp. +notifier: + # You can disable the notifier startup check by setting this to true + disable_startup_check: false +``` diff --git a/docs/configuration/notifier/smtp.md b/docs/configuration/notifier/smtp.md index 5800df1ce..5716fe422 100644 --- a/docs/configuration/notifier/smtp.md +++ b/docs/configuration/notifier/smtp.md @@ -16,9 +16,12 @@ It can be configured as described below. # # Notifications are sent to users when they require a password reset, a u2f # registration or a TOTP registration. -# Use only an available configuration: filesystem, smtp +# Use only an available configuration: filesystem, smtp. notifier: - # For testing purpose, notifications can be sent in a file + # You can disable the notifier startup check by setting this to true. + disable_startup_check: false + + # For testing purpose, notifications can be sent in a file. ## filesystem: ## filename: /tmp/authelia/notification.txt @@ -43,9 +46,11 @@ notifier: # Subject configuration of the emails sent. # {title} is replaced by the text from the notifier subject: "[Authelia] {title}" + # This address is used during the startup check to verify the email configuration is correct. It's not important what it is except if your email server only allows local delivery. + ## startup_check_address: test@authelia.com + ## trusted_cert: "" ## disable_require_tls: false ## disable_verify_cert: false - ## trusted_cert: "" ``` ## Using Gmail diff --git a/internal/configuration/schema/notifier.go b/internal/configuration/schema/notifier.go index 99d20bb2a..eb3877140 100644 --- a/internal/configuration/schema/notifier.go +++ b/internal/configuration/schema/notifier.go @@ -7,21 +7,23 @@ type FileSystemNotifierConfiguration struct { // SMTPNotifierConfiguration represents the configuration of the SMTP server to send emails with. type SMTPNotifierConfiguration struct { - Username string `mapstructure:"username"` - Password string `mapstructure:"password"` - Sender string `mapstructure:"sender"` - Subject string `mapstructure:"subject"` - Host string `mapstructure:"host"` - Port int `mapstructure:"port"` - TrustedCert string `mapstructure:"trusted_cert"` - DisableVerifyCert bool `mapstructure:"disable_verify_cert"` - DisableRequireTLS bool `mapstructure:"disable_require_tls"` + Host string `mapstructure:"host"` + Port int `mapstructure:"port"` + Username string `mapstructure:"username"` + Password string `mapstructure:"password"` + Sender string `mapstructure:"sender"` + Subject string `mapstructure:"subject"` + TrustedCert string `mapstructure:"trusted_cert"` + StartupCheckAddress string `mapstructure:"startup_check_address"` + DisableVerifyCert bool `mapstructure:"disable_verify_cert"` + DisableRequireTLS bool `mapstructure:"disable_require_tls"` } // NotifierConfiguration represents the configuration of the notifier to use when sending notifications to users. type NotifierConfiguration struct { - FileSystem *FileSystemNotifierConfiguration `mapstructure:"filesystem"` - SMTP *SMTPNotifierConfiguration `mapstructure:"smtp"` + DisableStartupCheck bool `mapstructure:"disable_startup_check"` + FileSystem *FileSystemNotifierConfiguration `mapstructure:"filesystem"` + SMTP *SMTPNotifierConfiguration `mapstructure:"smtp"` } // DefaultSMTPNotifierConfiguration represents default configuration parameters for the SMTP notifier. diff --git a/internal/configuration/validator/notifier.go b/internal/configuration/validator/notifier.go index f51e37254..8227506aa 100644 --- a/internal/configuration/validator/notifier.go +++ b/internal/configuration/validator/notifier.go @@ -26,6 +26,9 @@ func ValidateNotifier(configuration *schema.NotifierConfiguration, validator *sc } if configuration.SMTP != nil { + if configuration.SMTP.StartupCheckAddress == "" { + configuration.SMTP.StartupCheckAddress = "test@authelia.com" + } if configuration.SMTP.Host == "" { validator.Push(fmt.Errorf("Host of SMTP notifier must be provided")) } diff --git a/internal/mocks/mock_notifier.go b/internal/mocks/mock_notifier.go index 55a076d2c..d8569afce 100644 --- a/internal/mocks/mock_notifier.go +++ b/internal/mocks/mock_notifier.go @@ -10,30 +10,35 @@ import ( gomock "github.com/golang/mock/gomock" ) -// MockNotifier is a mock of Notifier interface +// MockNotifier is a mock of Notifier interface. type MockNotifier struct { ctrl *gomock.Controller recorder *MockNotifierMockRecorder } -// MockNotifierMockRecorder is the mock recorder for MockNotifier +// MockNotifierMockRecorder is the mock recorder for MockNotifier. type MockNotifierMockRecorder struct { mock *MockNotifier } -// NewMockNotifier creates a new mock instance +// NewMockNotifier creates a new mock instance. func NewMockNotifier(ctrl *gomock.Controller) *MockNotifier { mock := &MockNotifier{ctrl: ctrl} mock.recorder = &MockNotifierMockRecorder{mock} return mock } -// EXPECT returns an object that allows the caller to indicate expected use +// EXPECT returns an object that allows the caller to indicate expected use. func (m *MockNotifier) EXPECT() *MockNotifierMockRecorder { return m.recorder } -// Send mocks base method +// StartupCheck mocks base method. +func (m *MockNotifier) StartupCheck() (bool, error) { + return true, nil +} + +// Send mocks base method. func (m *MockNotifier) Send(arg0, arg1, arg2 string) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Send", arg0, arg1, arg2) @@ -41,7 +46,7 @@ func (m *MockNotifier) Send(arg0, arg1, arg2 string) error { return ret0 } -// Send indicates an expected call of Send +// Send indicates an expected call of Send. func (mr *MockNotifierMockRecorder) Send(arg0, arg1, arg2 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Send", reflect.TypeOf((*MockNotifier)(nil).Send), arg0, arg1, arg2) diff --git a/internal/notification/const.go b/internal/notification/const.go new file mode 100644 index 000000000..2421ff9cc --- /dev/null +++ b/internal/notification/const.go @@ -0,0 +1,3 @@ +package notification + +const fileNotifierMode = 0755 diff --git a/internal/notification/file_notifier.go b/internal/notification/file_notifier.go index 2e036ba93..10682fcb0 100644 --- a/internal/notification/file_notifier.go +++ b/internal/notification/file_notifier.go @@ -3,6 +3,8 @@ package notification import ( "fmt" "io/ioutil" + "os" + "path/filepath" "time" "github.com/authelia/authelia/internal/configuration/schema" @@ -20,11 +22,44 @@ func NewFileNotifier(configuration schema.FileSystemNotifierConfiguration) *File } } +// StartupCheck checks the file provider can write to the specified file +func (n *FileNotifier) StartupCheck() (ok bool, err error) { + ok = true + dir := filepath.Dir(n.path) + if _, err = os.Stat(dir); err != nil { + if os.IsNotExist(err) { + if err = os.MkdirAll(dir, fileNotifierMode); err != nil { + ok = false + return + } + if err = ioutil.WriteFile(n.path, []byte(""), fileNotifierMode); err != nil { + ok = false + return + } + } else { + ok = false + return + } + } else if _, err = os.Stat(n.path); err != nil { + if os.IsNotExist(err) { + if err = ioutil.WriteFile(n.path, []byte(""), fileNotifierMode); err != nil { + ok = false + return + } + } else { + ok = false + return + } + } + err = nil + return +} + // Send send a identity verification link to a user. func (n *FileNotifier) Send(recipient, subject, body string) error { content := fmt.Sprintf("Date: %s\nRecipient: %s\nSubject: %s\nBody: %s", time.Now(), recipient, subject, body) - err := ioutil.WriteFile(n.path, []byte(content), 0755) + err := ioutil.WriteFile(n.path, []byte(content), fileNotifierMode) if err != nil { return err diff --git a/internal/notification/notifier.go b/internal/notification/notifier.go index d829261d7..25c71db1e 100644 --- a/internal/notification/notifier.go +++ b/internal/notification/notifier.go @@ -3,4 +3,5 @@ package notification // Notifier interface for sending the identity verification link. type Notifier interface { Send(recipient, subject, body string) error + StartupCheck() (bool, error) } diff --git a/internal/notification/smtp_notifier.go b/internal/notification/smtp_notifier.go index 28a40abe3..ec57ab5b8 100644 --- a/internal/notification/smtp_notifier.go +++ b/internal/notification/smtp_notifier.go @@ -15,35 +15,37 @@ import ( "github.com/authelia/authelia/internal/utils" ) -// SMTPNotifier a notifier to send emails to SMTP servers +// SMTPNotifier a notifier to send emails to SMTP servers. type SMTPNotifier struct { - username string - password string - sender string - host string - port int - trustedCert string - disableVerifyCert bool - disableRequireTLS bool - address string - subject string - client *smtp.Client - tlsConfig *tls.Config + username string + password string + sender string + host string + port int + trustedCert string + disableVerifyCert bool + disableRequireTLS bool + address string + subject string + startupCheckAddress string + client *smtp.Client + tlsConfig *tls.Config } -// NewSMTPNotifier create an SMTPNotifier targeting a given address +// NewSMTPNotifier creates a SMTPNotifier using the notifier configuration. func NewSMTPNotifier(configuration schema.SMTPNotifierConfiguration) *SMTPNotifier { notifier := &SMTPNotifier{ - username: configuration.Username, - password: configuration.Password, - sender: configuration.Sender, - host: configuration.Host, - port: configuration.Port, - trustedCert: configuration.TrustedCert, - disableVerifyCert: configuration.DisableVerifyCert, - disableRequireTLS: configuration.DisableRequireTLS, - address: fmt.Sprintf("%s:%d", configuration.Host, configuration.Port), - subject: configuration.Subject, + username: configuration.Username, + password: configuration.Password, + sender: configuration.Sender, + host: configuration.Host, + port: configuration.Port, + trustedCert: configuration.TrustedCert, + disableVerifyCert: configuration.DisableVerifyCert, + disableRequireTLS: configuration.DisableRequireTLS, + address: fmt.Sprintf("%s:%d", configuration.Host, configuration.Port), + subject: configuration.Subject, + startupCheckAddress: configuration.StartupCheckAddress, } notifier.initializeTLSConfig() return notifier @@ -53,10 +55,6 @@ func (n *SMTPNotifier) initializeTLSConfig() { // Do not allow users to disable verification of certs if they have also set a trusted cert that was loaded // The second part of this check happens in the Configure Cert Pool code block log.Debug("Notifier SMTP client initializing TLS configuration") - insecureSkipVerify := false - if n.disableVerifyCert { - insecureSkipVerify = true - } //Configure Cert Pool certPool, err := x509.SystemCertPool() @@ -77,7 +75,7 @@ func (n *SMTPNotifier) initializeTLSConfig() { log.Debug("Notifier SMTP successfully loaded certificate") if n.disableVerifyCert { log.Warn("Notifier SMTP when trusted_cert is specified we force disable_verify_cert to false, if you want to disable certificate validation please comment/delete trusted_cert from your config") - insecureSkipVerify = false + n.disableVerifyCert = false } } } @@ -86,13 +84,13 @@ func (n *SMTPNotifier) initializeTLSConfig() { } } n.tlsConfig = &tls.Config{ - InsecureSkipVerify: insecureSkipVerify, + InsecureSkipVerify: n.disableVerifyCert, //nolint:gosec // This is an intended config, we never default true, provide alternate options, and we constantly warn the user. ServerName: n.host, RootCAs: certPool, } } -// Do startTLS if available (some servers only provide the auth extension after, and encryption is preferred) +// Do startTLS if available (some servers only provide the auth extension after, and encryption is preferred). func (n *SMTPNotifier) startTLS() (bool, error) { // Only start if not already encrypted if _, ok := n.client.TLSConnectionState(); ok { @@ -117,23 +115,23 @@ func (n *SMTPNotifier) startTLS() (bool, error) { return ok, nil } -// Attempt Authentication +// Attempt Authentication. func (n *SMTPNotifier) auth() (bool, error) { - // Attempt AUTH if password is specified only + // Attempt AUTH if password is specified only. if n.password != "" { _, ok := n.client.TLSConnectionState() if !ok { return false, errors.New("Notifier SMTP client does not support authentication over plain text and the connection is currently plain text") } - // Check the server supports AUTH, and get the mechanisms + // Check the server supports AUTH, and get the mechanisms. ok, m := n.client.Extension("AUTH") if ok { log.Debugf("Notifier SMTP server supports authentication with the following mechanisms: %s", m) mechanisms := strings.Split(m, " ") var auth smtp.Auth - // Adaptively select the AUTH mechanism to use based on what the server advertised + // Adaptively select the AUTH mechanism to use based on what the server advertised. if utils.IsStringInSlice("PLAIN", mechanisms) { auth = smtp.PlainAuth("", n.username, n.password, n.host) log.Debug("Notifier SMTP client attempting AUTH PLAIN with server") @@ -142,12 +140,12 @@ func (n *SMTPNotifier) auth() (bool, error) { log.Debug("Notifier SMTP client attempting AUTH LOGIN with server") } - // Throw error since AUTH extension is not supported + // Throw error since AUTH extension is not supported. if auth == nil { return false, fmt.Errorf("notifier SMTP server does not advertise a AUTH mechanism that are supported by Authelia (PLAIN or LOGIN are supported, but server advertised %s mechanisms)", m) } - // Authenticate + // Authenticate. err := n.client.Auth(auth) if err != nil { return false, err @@ -195,7 +193,7 @@ func (n *SMTPNotifier) compose(recipient, subject, body string) error { return nil } -// Dial the SMTP server with the SMTPNotifier config +// Dial the SMTP server with the SMTPNotifier config. func (n *SMTPNotifier) dial() error { log.Debugf("Notifier SMTP client attempting connection to %s", n.address) if n.port == 465 { @@ -220,7 +218,7 @@ func (n *SMTPNotifier) dial() error { return nil } -// Closes the connection properly +// Closes the connection properly. func (n *SMTPNotifier) cleanup() { err := n.client.Quit() if err != nil { @@ -228,17 +226,56 @@ func (n *SMTPNotifier) cleanup() { } } -// Send an email +// StartupCheck checks the server is functioning correctly and the configuration is correct. +func (n *SMTPNotifier) StartupCheck() (ok bool, err error) { + ok = true + + if err = n.dial(); err != nil { + ok = false + return + } + + defer n.cleanup() + + if _, err = n.startTLS(); err != nil { + ok = false + return + } + + if _, err = n.auth(); err != nil { + ok = false + return + } + + if err = n.client.Mail(n.sender); err != nil { + ok = false + return + } + + if err = n.client.Rcpt(n.startupCheckAddress); err != nil { + ok = false + return + } + + if err = n.client.Reset(); err != nil { + ok = false + return + } + + return +} + +// Send is used to send an email to a recipient. func (n *SMTPNotifier) Send(recipient, title, body string) error { subject := strings.ReplaceAll(n.subject, "{title}", title) if err := n.dial(); err != nil { return err } - // Always execute QUIT at the end once we're connected + // Always execute QUIT at the end once we're connected. defer n.cleanup() - // Start TLS and then Authenticate + // Start TLS and then Authenticate. if _, err := n.startTLS(); err != nil { return err } @@ -246,7 +283,7 @@ func (n *SMTPNotifier) Send(recipient, title, body string) error { return err } - // Set the sender and recipient first + // Set the sender and recipient first. if err := n.client.Mail(n.sender); err != nil { log.Debugf("Notifier SMTP failed while sending MAIL FROM (using sender) with error: %s", err) return err @@ -256,7 +293,7 @@ func (n *SMTPNotifier) Send(recipient, title, body string) error { return err } - // Compose and send the email body to the server + // Compose and send the email body to the server. if err := n.compose(recipient, subject, body); err != nil { return err } diff --git a/internal/suites/DuoPush/configuration.yml b/internal/suites/DuoPush/configuration.yml index 4648839dd..60451bf8d 100644 --- a/internal/suites/DuoPush/configuration.yml +++ b/internal/suites/DuoPush/configuration.yml @@ -97,9 +97,5 @@ regulation: ban_time: 900 notifier: - # Use a SMTP server for sending notifications - smtp: - host: smtp - port: 1025 - sender: admin@example.com - disable_require_tls: true \ No newline at end of file + filesystem: + filename: /tmp/notifier.html \ No newline at end of file diff --git a/internal/suites/OneFactorOnly/configuration.yml b/internal/suites/OneFactorOnly/configuration.yml index 61b84f20e..c82ace09a 100644 --- a/internal/suites/OneFactorOnly/configuration.yml +++ b/internal/suites/OneFactorOnly/configuration.yml @@ -40,8 +40,5 @@ access_control: policy: bypass notifier: - smtp: - host: smtp - port: 1025 - sender: admin@example.com - disable_require_tls: true \ No newline at end of file + filesystem: + filename: /tmp/notifier.html \ No newline at end of file