From 997036f9c32979ca0de5be2deff1f1cb2565938a Mon Sep 17 00:00:00 2001 From: James Elliott Date: Sat, 7 Aug 2021 13:58:08 +1000 Subject: [PATCH] fix(configuration): make notifier logging consistent and more specific (#2268) This ensures the notifier logs are more specific to give people a clear picture of if they either have no notifier specified or multiple. --- docs/configuration/migration.md | 5 +++++ internal/configuration/validator/const.go | 10 ++++++++++ internal/configuration/validator/notifier.go | 17 ++++++++++------- .../configuration/validator/notifier_test.go | 13 +++++++------ 4 files changed, 32 insertions(+), 13 deletions(-) diff --git a/docs/configuration/migration.md b/docs/configuration/migration.md index c9b7ec04c..5de1bdb1a 100644 --- a/docs/configuration/migration.md +++ b/docs/configuration/migration.md @@ -42,6 +42,11 @@ The following changes occurred in 4.30.0: |log_file_path|log.file_path | |log_format |log.format | +_**Please Note:** you can no longer define secrets for providers that you are not using. For example if you're using the +[filesystem notifier](./notifier/filesystem.md) you must ensure that the `AUTHELIA_NOTIFIER_SMTP_PASSWORD_FILE` +environment variable or other environment variables set. This also applies to other providers like +[storage](./storage/index.md) and [authentication backend](./authentication/index.md)._ + #### Kubernetes 4.30.0 _**Please Note:** if you're using Authelia with Kubernetes and are not using the provided [helm chart](https://charts.authelia.com) diff --git a/internal/configuration/validator/const.go b/internal/configuration/validator/const.go index 55006f4bc..95c7d2c7d 100644 --- a/internal/configuration/validator/const.go +++ b/internal/configuration/validator/const.go @@ -43,6 +43,16 @@ const ( testTLSKey = "/tmp/key.pem" ) +// Notifier Error constants. +const ( + errFmtNotifierMultipleConfigured = "notifier: you can't configure more than one notifier, please ensure " + + "only 'smtp' or 'filesystem' is configured" + errFmtNotifierNotConfigured = "notifier: you must ensure either the 'smtp' or 'filesystem' notifier " + + "is configured" + errFmtNotifierFileSystemFileNameNotConfigured = "filesystem notifier: the 'filename' must be configured" + errFmtNotifierSMTPNotConfigured = "smtp notifier: the '%s' must be configured" +) + // OpenID Error constants. const ( errFmtOIDCClientsDuplicateID = "openid connect provider: one or more clients have the same ID" diff --git a/internal/configuration/validator/notifier.go b/internal/configuration/validator/notifier.go index 38c3798d8..4ac7fd387 100644 --- a/internal/configuration/validator/notifier.go +++ b/internal/configuration/validator/notifier.go @@ -8,16 +8,19 @@ import ( // ValidateNotifier validates and update notifier configuration. func ValidateNotifier(configuration *schema.NotifierConfiguration, validator *schema.StructValidator) { - if configuration.SMTP == nil && configuration.FileSystem == nil || - configuration.SMTP != nil && configuration.FileSystem != nil { - validator.Push(fmt.Errorf("Notifier should be either `smtp` or `filesystem`")) + if configuration.SMTP == nil && configuration.FileSystem == nil { + validator.Push(fmt.Errorf(errFmtNotifierNotConfigured)) + + return + } else if configuration.SMTP != nil && configuration.FileSystem != nil { + validator.Push(fmt.Errorf(errFmtNotifierMultipleConfigured)) return } if configuration.FileSystem != nil { if configuration.FileSystem.Filename == "" { - validator.Push(fmt.Errorf("Filename of filesystem notifier must not be empty")) + validator.Push(fmt.Errorf(errFmtNotifierFileSystemFileNameNotConfigured)) } return @@ -32,15 +35,15 @@ func validateSMTPNotifier(configuration *schema.SMTPNotifierConfiguration, valid } if configuration.Host == "" { - validator.Push(fmt.Errorf("Host of SMTP notifier must be provided")) + validator.Push(fmt.Errorf(errFmtNotifierSMTPNotConfigured, "host")) } if configuration.Port == 0 { - validator.Push(fmt.Errorf("Port of SMTP notifier must be provided")) + validator.Push(fmt.Errorf(errFmtNotifierSMTPNotConfigured, "port")) } if configuration.Sender == "" { - validator.Push(fmt.Errorf("Sender of SMTP notifier must be provided")) + validator.Push(fmt.Errorf(errFmtNotifierSMTPNotConfigured, "sender")) } if configuration.Subject == "" { diff --git a/internal/configuration/validator/notifier_test.go b/internal/configuration/validator/notifier_test.go index b26f010d6..54097d206 100644 --- a/internal/configuration/validator/notifier_test.go +++ b/internal/configuration/validator/notifier_test.go @@ -1,6 +1,7 @@ package validator import ( + "fmt" "testing" "github.com/stretchr/testify/suite" @@ -40,7 +41,7 @@ func (suite *NotifierSuite) TestShouldEnsureAtLeastSMTPOrFilesystemIsProvided() suite.Assert().Len(suite.validator.Errors(), 1) - suite.Assert().EqualError(suite.validator.Errors()[0], "Notifier should be either `smtp` or `filesystem`") + suite.Assert().EqualError(suite.validator.Errors()[0], errFmtNotifierNotConfigured) } func (suite *NotifierSuite) TestShouldEnsureEitherSMTPOrFilesystemIsProvided() { @@ -59,7 +60,7 @@ func (suite *NotifierSuite) TestShouldEnsureEitherSMTPOrFilesystemIsProvided() { suite.Assert().Len(suite.validator.Errors(), 1) - suite.Assert().EqualError(suite.validator.Errors()[0], "Notifier should be either `smtp` or `filesystem`") + suite.Assert().EqualError(suite.validator.Errors()[0], errFmtNotifierMultipleConfigured) } func (suite *NotifierSuite) TestShouldEnsureFilenameOfFilesystemNotifierIsProvided() { @@ -81,7 +82,7 @@ func (suite *NotifierSuite) TestShouldEnsureFilenameOfFilesystemNotifierIsProvid suite.Assert().Len(suite.validator.Errors(), 1) - suite.Assert().EqualError(suite.validator.Errors()[0], "Filename of filesystem notifier must not be empty") + suite.Assert().EqualError(suite.validator.Errors()[0], errFmtNotifierFileSystemFileNameNotConfigured) } func (suite *NotifierSuite) TestShouldEnsureHostAndPortOfSMTPNotifierAreProvided() { @@ -103,8 +104,8 @@ func (suite *NotifierSuite) TestShouldEnsureHostAndPortOfSMTPNotifierAreProvided suite.Require().Len(errors, 2) - suite.Assert().EqualError(errors[0], "Host of SMTP notifier must be provided") - suite.Assert().EqualError(errors[1], "Port of SMTP notifier must be provided") + suite.Assert().EqualError(errors[0], fmt.Sprintf(errFmtNotifierSMTPNotConfigured, "host")) + suite.Assert().EqualError(errors[1], fmt.Sprintf(errFmtNotifierSMTPNotConfigured, "port")) } func (suite *NotifierSuite) TestShouldEnsureSenderOfSMTPNotifierAreProvided() { @@ -124,7 +125,7 @@ func (suite *NotifierSuite) TestShouldEnsureSenderOfSMTPNotifierAreProvided() { suite.Assert().Len(suite.validator.Errors(), 1) - suite.Assert().EqualError(suite.validator.Errors()[0], "Sender of SMTP notifier must be provided") + suite.Assert().EqualError(suite.validator.Errors()[0], fmt.Sprintf(errFmtNotifierSMTPNotConfigured, "sender")) } func TestNotifierSuite(t *testing.T) {