From 6ccb96ff745b4fec8c5187994d8f570c64170921 Mon Sep 17 00:00:00 2001 From: Sabith K Soopy Date: Fri, 27 Jul 2018 12:51:30 -0700 Subject: [PATCH 1/2] Add some test to validate the configuration --- cmd/dex/config.go | 33 +++++++++++++++++++++++++++++++++ cmd/dex/config_test.go | 41 +++++++++++++++++++++++++++++++++++++++++ cmd/dex/serve.go | 24 ++---------------------- 3 files changed, 76 insertions(+), 22 deletions(-) diff --git a/cmd/dex/config.go b/cmd/dex/config.go index ed1cc9f2..7c0e9b5e 100644 --- a/cmd/dex/config.go +++ b/cmd/dex/config.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "os" + "strings" "golang.org/x/crypto/bcrypt" @@ -48,6 +49,38 @@ type Config struct { StaticPasswords []password `json:"staticPasswords"` } +//Validate the configuration +func (c Config) Validate() error { + // Fast checks. Perform these first for a more responsive CLI. + checks := []struct { + bad bool + errMsg string + }{ + {c.Issuer == "", "no issuer specified in config file"}, + {!c.EnablePasswordDB && len(c.StaticPasswords) != 0, "cannot specify static passwords without enabling password db"}, + {c.Storage.Config == nil, "no storage supplied in config file"}, + {c.Web.HTTP == "" && c.Web.HTTPS == "", "must supply a HTTP/HTTPS address to listen on"}, + {c.Web.HTTPS != "" && c.Web.TLSCert == "", "no cert specified for HTTPS"}, + {c.Web.HTTPS != "" && c.Web.TLSKey == "", "no private key specified for HTTPS"}, + {c.GRPC.TLSCert != "" && c.GRPC.Addr == "", "no address specified for gRPC"}, + {c.GRPC.TLSKey != "" && c.GRPC.Addr == "", "no address specified for gRPC"}, + {(c.GRPC.TLSCert == "") != (c.GRPC.TLSKey == ""), "must specific both a gRPC TLS cert and key"}, + {c.GRPC.TLSCert == "" && c.GRPC.TLSClientCA != "", "cannot specify gRPC TLS client CA without a gRPC TLS cert"}, + } + + var checkErrors []string + + for _, check := range checks { + if check.bad { + checkErrors = append(checkErrors, check.errMsg) + } + } + if len(checkErrors) != 0 { + return fmt.Errorf("Invalid Config:\n\t-\t%s", strings.Join(checkErrors, "\n\t-\t")) + } + return nil +} + type password storage.Password func (p *password) UnmarshalJSON(b []byte) error { diff --git a/cmd/dex/config_test.go b/cmd/dex/config_test.go index 701b8ae7..90361cd6 100644 --- a/cmd/dex/config_test.go +++ b/cmd/dex/config_test.go @@ -14,6 +14,47 @@ import ( var _ = yaml.YAMLToJSON +func TestValidConfiguration(t *testing.T) { + configuration := Config{ + Issuer: "http://127.0.0.1:5556/dex", + Storage: Storage{ + Type: "sqlite3", + Config: &sql.SQLite3{ + File: "examples/dex.db", + }, + }, + Web: Web{ + HTTP: "127.0.0.1:5556", + }, + StaticConnectors: []Connector{ + { + Type: "mockCallback", + ID: "mock", + Name: "Example", + Config: &mock.CallbackConfig{}, + }, + }, + } + if err := configuration.Validate(); err != nil { + t.Fatalf("this configuration should have been valid: %v", err) + } +} + +func TestInvalidConfiguration(t *testing.T) { + configuration := Config{} + err := configuration.Validate() + if err == nil { + t.Fatal("this configuration should be invalid") + } + got := err.Error() + wanted := `Invalid Config: + - no issuer specified in config file + - no storage supplied in config file + - must supply a HTTP/HTTPS address to listen on` + if got != wanted { + t.Fatalf("Expected error message to be %q, got %q", wanted, got) + } +} func TestUnmarshalConfig(t *testing.T) { rawConfig := []byte(` issuer: http://127.0.0.1:5556/dex diff --git a/cmd/dex/serve.go b/cmd/dex/serve.go index 441cbe64..91b0fd22 100644 --- a/cmd/dex/serve.go +++ b/cmd/dex/serve.go @@ -71,28 +71,8 @@ func serve(cmd *cobra.Command, args []string) error { if c.Logger.Level != "" { logger.Infof("config using log level: %s", c.Logger.Level) } - - // Fast checks. Perform these first for a more responsive CLI. - checks := []struct { - bad bool - errMsg string - }{ - {c.Issuer == "", "no issuer specified in config file"}, - {!c.EnablePasswordDB && len(c.StaticPasswords) != 0, "cannot specify static passwords without enabling password db"}, - {c.Storage.Config == nil, "no storage supplied in config file"}, - {c.Web.HTTP == "" && c.Web.HTTPS == "", "must supply a HTTP/HTTPS address to listen on"}, - {c.Web.HTTPS != "" && c.Web.TLSCert == "", "no cert specified for HTTPS"}, - {c.Web.HTTPS != "" && c.Web.TLSKey == "", "no private key specified for HTTPS"}, - {c.GRPC.TLSCert != "" && c.GRPC.Addr == "", "no address specified for gRPC"}, - {c.GRPC.TLSKey != "" && c.GRPC.Addr == "", "no address specified for gRPC"}, - {(c.GRPC.TLSCert == "") != (c.GRPC.TLSKey == ""), "must specific both a gRPC TLS cert and key"}, - {c.GRPC.TLSCert == "" && c.GRPC.TLSClientCA != "", "cannot specify gRPC TLS client CA without a gRPC TLS cert"}, - } - - for _, check := range checks { - if check.bad { - return fmt.Errorf("invalid config: %s", check.errMsg) - } + if err := c.Validate(); err != nil { + return err } logger.Infof("config issuer: %s", c.Issuer) From 6769a3b18e37143da55dc4db21e037597ac266f2 Mon Sep 17 00:00:00 2001 From: Sabith K Soopy Date: Mon, 29 Apr 2019 07:33:10 -0700 Subject: [PATCH 2/2] Errors should not start with caps - https://github.com/dexidp/dex/pull/1264#discussion_r253264017 Signed-off-by: Sabith --- cmd/dex/config.go | 2 +- cmd/dex/config_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/dex/config.go b/cmd/dex/config.go index 7c0e9b5e..882bfd1d 100644 --- a/cmd/dex/config.go +++ b/cmd/dex/config.go @@ -76,7 +76,7 @@ func (c Config) Validate() error { } } if len(checkErrors) != 0 { - return fmt.Errorf("Invalid Config:\n\t-\t%s", strings.Join(checkErrors, "\n\t-\t")) + return fmt.Errorf("invalid Config:\n\t-\t%s", strings.Join(checkErrors, "\n\t-\t")) } return nil } diff --git a/cmd/dex/config_test.go b/cmd/dex/config_test.go index 90361cd6..77c4874a 100644 --- a/cmd/dex/config_test.go +++ b/cmd/dex/config_test.go @@ -47,7 +47,7 @@ func TestInvalidConfiguration(t *testing.T) { t.Fatal("this configuration should be invalid") } got := err.Error() - wanted := `Invalid Config: + wanted := `invalid Config: - no issuer specified in config file - no storage supplied in config file - must supply a HTTP/HTTPS address to listen on`