From a9b0caf4ee9569436cf285151e1bdcff97a04bfc Mon Sep 17 00:00:00 2001 From: Amir Zarrinkafsh Date: Tue, 9 Jun 2020 08:22:41 +1000 Subject: [PATCH] [MISC] Catch and warn on malformed configuration yaml (#1089) If the configuration yaml is poorly indented or special values are not appropriately escaped Authelia attempts to load said configuration and fails. This attempts to unmarshal the config into an empty interface to catch and warn on malformed yaml. Using the example from issue https://github.com/authelia/authelia/issues/1053#issuecomment-634791662 ```yaml host: 0.0.0.0 port: 9091 log_level: debug jwt_secret: RUtG9TnbXrOl1XLLmDgySw1DGgx9QcrtepIf1uDDBlBVKFZxkVBruYKBi32PvaU default_redirection_url: example.com totp: issuer: example.com period: 30 skew: 1 authentication_backend: file: path: /etc/authelia/users_database.yml access_control: default_policy: deny rules: - domain: example.com policy: bypass - domain: "*.example.com" policy: one_factor session: name: authelia_session secret: TVPMIcDFbBwhnW3kLJzKhdjeHhtqisr7m28FgRY8oLh2A4lwuV2jV2ZGdGbh4aa expiration: 3600 inactivity: 300 domain: example.com regulation: max_retries: 3 find_time: 120 ban_time: 300 storage: mysql: host: example.com port: 3306 database: authelia username: authelia password: example.com notifier: smtp: username: example.com password: example.com host: smtp.gmail.com port: 465 sender: example.com ``` We would actually get a more meaningful error which helps pinpoint the issue: `Error malformed yaml: line 23: did not find expected alphabetic or numeric character` --- .../{main_test.go => coverage_test.go} | 2 +- cmd/authelia/main.go | 7 +-- internal/configuration/reader.go | 31 +++++++++-- internal/configuration/reader_test.go | 33 ++++++++++++ .../test_resources/config_bad_quoting.yml | 52 +++++++++++++++++++ 5 files changed, 113 insertions(+), 12 deletions(-) rename cmd/authelia/{main_test.go => coverage_test.go} (94%) create mode 100644 internal/configuration/test_resources/config_bad_quoting.yml diff --git a/cmd/authelia/main_test.go b/cmd/authelia/coverage_test.go similarity index 94% rename from cmd/authelia/main_test.go rename to cmd/authelia/coverage_test.go index f50e49ad..26d7fe02 100644 --- a/cmd/authelia/main_test.go +++ b/cmd/authelia/coverage_test.go @@ -10,7 +10,7 @@ import ( "testing" ) -func TestAuthelia(t *testing.T) { +func TestCoverage(t *testing.T) { var ( args []string ) diff --git a/cmd/authelia/main.go b/cmd/authelia/main.go index 6ada8e1a..64cdd02d 100644 --- a/cmd/authelia/main.go +++ b/cmd/authelia/main.go @@ -1,7 +1,6 @@ package main import ( - "errors" "fmt" "log" "os" @@ -27,10 +26,6 @@ var configPathFlag string //nolint:gocyclo // TODO: Consider refactoring/simplifying, time permitting func startServer() { - if configPathFlag == "" { - log.Fatal(errors.New("No config file path provided")) - } - config, errs := configuration.Read(configPathFlag) if len(errs) > 0 { @@ -38,7 +33,7 @@ func startServer() { logging.Logger().Error(err) } - panic(errors.New("Some errors have been reported")) + os.Exit(1) } if err := logging.InitializeLogger(config.LogFilePath); err != nil { diff --git a/internal/configuration/reader.go b/internal/configuration/reader.go index 16a5963b..e89f7a8c 100644 --- a/internal/configuration/reader.go +++ b/internal/configuration/reader.go @@ -1,10 +1,14 @@ package configuration import ( + "errors" "fmt" + "io/ioutil" + "os" "strings" "github.com/spf13/viper" + "gopkg.in/yaml.v2" "github.com/authelia/authelia/internal/configuration/schema" "github.com/authelia/authelia/internal/configuration/validator" @@ -12,6 +16,27 @@ import ( // Read a YAML configuration and create a Configuration object out of it. func Read(configPath string) (*schema.Configuration, []error) { + if configPath == "" { + return nil, []error{errors.New("No config file path provided")} + } + + _, err := os.Stat(configPath) + if err != nil { + return nil, []error{fmt.Errorf("Unable to find config file: %v", configPath)} + } + + file, err := ioutil.ReadFile(configPath) + if err != nil { + return nil, []error{fmt.Errorf("Failed to %v", err)} + } + + var data interface{} + + err = yaml.Unmarshal(file, &data) + if err != nil { + return nil, []error{fmt.Errorf("Error malformed %v", err)} + } + viper.SetEnvKeyReplacer(strings.NewReplacer(".", "_")) viper.BindEnv("authelia.jwt_secret.file") //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting. @@ -25,11 +50,7 @@ func Read(configPath string) (*schema.Configuration, []error) { viper.SetConfigFile(configPath) - if err := viper.ReadInConfig(); err != nil { - if _, ok := err.(viper.ConfigFileNotFoundError); ok { - return nil, []error{fmt.Errorf("unable to find config file %s", configPath)} - } - } + _ = viper.ReadInConfig() var configuration schema.Configuration diff --git a/internal/configuration/reader_test.go b/internal/configuration/reader_test.go index ad8f42ab..ecc5bfcc 100644 --- a/internal/configuration/reader_test.go +++ b/internal/configuration/reader_test.go @@ -57,6 +57,39 @@ func setupEnv(t *testing.T) string { return dir } +func TestShouldErrorNoConfigPath(t *testing.T) { + _, errors := Read("") + + require.Len(t, errors, 1) + + require.EqualError(t, errors[0], "No config file path provided") +} + +func TestShouldErrorNoConfigFile(t *testing.T) { + _, errors := Read("./nonexistent.yml") + + require.Len(t, errors, 1) + + require.EqualError(t, errors[0], "Unable to find config file: ./nonexistent.yml") +} + +func TestShouldErrorPermissionsConfigFile(t *testing.T) { + _ = ioutil.WriteFile("/tmp/authelia/permissions.yml", []byte{}, 0000) // nolint:gosec + _, errors := Read("/tmp/authelia/permissions.yml") + + require.Len(t, errors, 1) + + require.EqualError(t, errors[0], "Failed to open /tmp/authelia/permissions.yml: permission denied") +} + +func TestShouldErrorParseBadConfigFile(t *testing.T) { + _, errors := Read("./test_resources/config_bad_quoting.yml") + + require.Len(t, errors, 1) + + require.EqualError(t, errors[0], "Error malformed yaml: line 23: did not find expected alphabetic or numeric character") +} + func TestShouldParseConfigFile(t *testing.T) { dir := setupEnv(t) diff --git a/internal/configuration/test_resources/config_bad_quoting.yml b/internal/configuration/test_resources/config_bad_quoting.yml new file mode 100644 index 00000000..1f41a8a5 --- /dev/null +++ b/internal/configuration/test_resources/config_bad_quoting.yml @@ -0,0 +1,52 @@ +host: 0.0.0.0 +port: 9091 +log_level: debug + +jwt_secret: RUtG9TnbXrOl1XLLmDgySw1DGgx9QcrtepIf1uDDBlBVKFZxkVBruYKBi32PvaU + +default_redirection_url: example.com + +totp: + issuer: example.com + period: 30 + skew: 1 + +authentication_backend: + file: + path: /etc/authelia/users_database.yml + +access_control: + default_policy: deny + rules: + - domain: example.com + policy: bypass + - domain: *.example.com + policy: one_factor + +session: + name: authelia_session + secret: TVPMIcDFbBwhnW3kLJzKhdjeHhtqisr7m28FgRY8oLh2A4lwuV2jV2ZGdGbh4aa + expiration: 3600 + inactivity: 300 + domain: example.com + +regulation: + max_retries: 3 + find_time: 120 + ban_time: 300 + +storage: + mysql: + host: example.com + port: 3306 + database: authelia + username: authelia + password: example.com + +notifier: + smtp: + username: example.com + password: example.com + host: smtp.gmail.com + port: 465 + sender: example.com \ No newline at end of file