From c7e81e2b18e96a970c1a3f8ba0a5cf965d5f0add Mon Sep 17 00:00:00 2001 From: Mateusz Filipowicz Date: Sun, 9 Feb 2025 04:21:15 +0100 Subject: [PATCH] feat: add validation of ClientConfig fields for improved data integrity (#5) * feat: add validation of ClientConfig fields for improved data integrity * chore: add tests for client config validation --- go.mod | 4 +++ go.sum | 10 ++++++ unifi/unifi.go | 21 ++++++++---- unifi/unifi_test.go | 84 +++++++++++++++++++++++++++++++++++++++++++-- unifi/validation.go | 77 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 188 insertions(+), 8 deletions(-) create mode 100644 unifi/validation.go diff --git a/go.mod b/go.mod index ba0c420..b87e9e8 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,9 @@ go 1.23 toolchain go1.23.5 require ( + github.com/go-playground/locales v0.14.1 + github.com/go-playground/universal-translator v0.18.1 + github.com/go-playground/validator/v10 v10.24.0 github.com/golangci/golangci-lint v1.63.4 github.com/goreleaser/goreleaser v1.26.2 github.com/hashicorp/go-version v1.7.0 @@ -283,6 +286,7 @@ require ( github.com/ldez/grignotin v0.7.0 // indirect github.com/ldez/tagliatelle v0.7.1 // indirect github.com/ldez/usetesting v0.4.2 // indirect + github.com/leodido/go-urn v1.4.0 // indirect github.com/leonklingele/grouper v1.1.2 // indirect github.com/letsencrypt/boulder v0.0.0-20231026200631-000cd05d5491 // indirect github.com/lucasb-eyer/go-colorful v1.2.0 // indirect diff --git a/go.sum b/go.sum index a2b49d6..c9149a5 100644 --- a/go.sum +++ b/go.sum @@ -409,6 +409,14 @@ github.com/go-openapi/swag v0.23.0 h1:vsEVJDUo2hPJ2tu0/Xc+4noaxyEffXNIs3cOULZ+Gr github.com/go-openapi/swag v0.23.0/go.mod h1:esZ8ITTYEsH1V2trKHjAN8Ai7xHb8RV+YSZ577vPjgQ= github.com/go-openapi/validate v0.24.0 h1:LdfDKwNbpB6Vn40xhTdNZAnfLECL81w+VX3BumrGD58= github.com/go-openapi/validate v0.24.0/go.mod h1:iyeX1sEufmv3nPbBdX3ieNviWnOZaJ1+zquzJEf2BAQ= +github.com/go-playground/assert/v2 v2.2.0 h1:JvknZsQTYeFEAhQwI4qEt9cyV5ONwRHC+lYKSsYSR8s= +github.com/go-playground/assert/v2 v2.2.0/go.mod h1:VDjEfimB/XKnb+ZQfWdccd7VUvScMdVu0Titje2rxJ4= +github.com/go-playground/locales v0.14.1 h1:EWaQ/wswjilfKLTECiXz7Rh+3BjFhfDFKv/oXslEjJA= +github.com/go-playground/locales v0.14.1/go.mod h1:hxrqLVvrK65+Rwrd5Fc6F2O76J/NuW9t0sjnWqG1slY= +github.com/go-playground/universal-translator v0.18.1 h1:Bcnm0ZwsGyWbCzImXv+pAJnYK9S473LQFuzCbDbfSFY= +github.com/go-playground/universal-translator v0.18.1/go.mod h1:xekY+UJKNuX9WP91TpwSH2VMlDf28Uj24BCp08ZFTUY= +github.com/go-playground/validator/v10 v10.24.0 h1:KHQckvo8G6hlWnrPX4NJJ+aBfWNAE/HH+qdL2cBpCmg= +github.com/go-playground/validator/v10 v10.24.0/go.mod h1:GGzBIJMuE98Ic/kJsBXbz1x/7cByt++cQ+YOuDM5wus= github.com/go-quicktest/qt v1.101.0 h1:O1K29Txy5P2OK0dGo59b7b0LR6wKfIhttaAhHUyn7eI= github.com/go-quicktest/qt v1.101.0/go.mod h1:14Bz/f7NwaXPtdYEgzsx46kqSxVwTbzVZsDC26tQJow= github.com/go-restruct/restruct v1.2.0-alpha h1:2Lp474S/9660+SJjpVxoKuWX09JsXHSrdV7Nv3/gkvc= @@ -708,6 +716,8 @@ github.com/ldez/tagliatelle v0.7.1 h1:bTgKjjc2sQcsgPiT902+aadvMjCeMHrY7ly2XKFORI github.com/ldez/tagliatelle v0.7.1/go.mod h1:3zjxUpsNB2aEZScWiZTHrAXOl1x25t3cRmzfK1mlo2I= github.com/ldez/usetesting v0.4.2 h1:J2WwbrFGk3wx4cZwSMiCQQ00kjGR0+tuuyW0Lqm4lwA= github.com/ldez/usetesting v0.4.2/go.mod h1:eEs46T3PpQ+9RgN9VjpY6qWdiw2/QmfiDeWmdZdrjIQ= +github.com/leodido/go-urn v1.4.0 h1:WT9HwE9SGECu3lg4d/dIA+jxlljEa1/ffXKmRjqdmIQ= +github.com/leodido/go-urn v1.4.0/go.mod h1:bvxc+MVxLKB4z00jd1z+Dvzr47oO32F/QSNjSBOlFxI= github.com/leonklingele/grouper v1.1.2 h1:o1ARBDLOmmasUaNDesWqWCIFH3u7hoFlM84YrjT3mIY= github.com/leonklingele/grouper v1.1.2/go.mod h1:6D0M/HVkhs2yRKRFZUoGjeDy7EZTfFBE9gl4kjmIGkA= github.com/letsencrypt/boulder v0.0.0-20231026200631-000cd05d5491 h1:WGrKdjHtWC67RX96eTkYD2f53NDHhrq/7robWTAfk4s= diff --git a/unifi/unifi.go b/unifi/unifi.go index 8072e65..1d378d4 100644 --- a/unifi/unifi.go +++ b/unifi/unifi.go @@ -85,10 +85,10 @@ func (m *Meta) error() error { } type ClientConfig struct { - User string - Pass string - APIKey string - URL string + URL string `validate:"required,http_url"` + APIKey string `validate:"required_without_all=User Pass"` + User string `validate:"excluded_with=APIKey,required_with=Pass"` + Pass string `validate:"excluded_with=APIKey,required_with=User"` Timeout time.Duration // how long to wait for replies, default: forever. VerifySSL bool Interceptors []ClientInterceptor @@ -107,6 +107,7 @@ type Client struct { interceptors []ClientInterceptor errorHandler ResponseErrorHandler lock sync.Mutex + validator *validator } type ApiPaths struct { @@ -240,7 +241,14 @@ func (d *DefaultResponseErrorHandler) HandleError(resp *http.Response) error { // Used to make additional, authenticated requests to the APIs. // Start here. func NewClient(config *ClientConfig) (*Client, error) { - u, err := newUnifi(config) + v, err := newValidator() + if err != nil { + return nil, fmt.Errorf("failed creating validator: %w", err) + } + if err := v.Validate(config); err != nil { + return nil, fmt.Errorf("failed validating config: %w", err) + } + u, err := newUnifi(config, v) if err != nil { return nil, fmt.Errorf("failed creating unifi client: %w", err) } @@ -275,7 +283,7 @@ func parseBaseUrl(base string) (*url.URL, error) { return baseURL, nil } -func newUnifi(config *ClientConfig) (*Client, error) { +func newUnifi(config *ClientConfig, v *validator) (*Client, error) { var err error config.URL = strings.TrimRight(config.URL, "/") @@ -337,6 +345,7 @@ func newUnifi(config *ClientConfig) (*Client, error) { interceptors: interceptors, errorHandler: errorHandler, lock: sync.Mutex{}, + validator: v, } for _, interceptor := range config.Interceptors { // add any custom interceptors and ensure no duplicates diff --git a/unifi/unifi_test.go b/unifi/unifi_test.go index f75ee4e..6006360 100644 --- a/unifi/unifi_test.go +++ b/unifi/unifi_test.go @@ -102,7 +102,8 @@ func TestCustomizeHttpClient(t *testing.T) { // when _, err := NewClient(&ClientConfig{ - URL: localUrl, + URL: localUrl, + APIKey: "test-key", HttpCustomizer: func(transport *http.Transport) error { called = true return nil @@ -439,7 +440,8 @@ func TestResponseDataHandling(t *testing.T) { } srv := RunTestServer(NewStyleAPI.ApiPath+"/test", TestData{}) c, _ := NewClient(&ClientConfig{ - URL: srv.URL, + URL: srv.URL, + APIKey: "test-key", }) c.apiPaths = &NewStyleAPI var data TestData @@ -460,6 +462,8 @@ func TestCsrfHandling(t *testing.T) { interceptor := NewTestInterceptor() c, _ := NewClient(&ClientConfig{ URL: srv.URL, + User: "test-user", + Pass: "test-pass", Interceptors: interceptor.AsList(), }) c.apiPaths = &NewStyleAPI @@ -487,6 +491,7 @@ func TestOverrideUserAgent(t *testing.T) { interceptor := NewTestInterceptor() c, _ := NewClient(&ClientConfig{ URL: testUrl, + APIKey: "test-key", Interceptors: interceptor.AsList(), UserAgent: "test-agent", }) @@ -499,3 +504,78 @@ func TestOverrideUserAgent(t *testing.T) { require.Error(t, err) a.EqualValues("test-agent", interceptor.RequestHeader(UserAgentHeader)) } + +func TestAuthConfigurationValidation(t *testing.T) { + t.Parallel() + testCases := []struct { + User, Pass, APIKey string + shouldFail bool + }{ + {"", "", "", true}, + {"", "", "test", false}, + {"", "test", "", true}, + {"", "test", "test", true}, + {"test", "", "", true}, + {"test", "", "test", true}, + {"test", "test", "", false}, + {"test", "test", "test", true}, + } + + for _, tc := range testCases { + t.Run(fmt.Sprintf("user:%s-pass:%s-apikey:%s", tc.User, tc.Pass, tc.APIKey), func(t *testing.T) { + t.Parallel() + // given + _, err := NewClient(&ClientConfig{ + URL: testUrl, + User: tc.User, + Pass: tc.Pass, + APIKey: tc.APIKey, + }) + + // then + if tc.shouldFail { + require.ErrorContains(t, err, "validation failed") + return + } + require.ErrorContains(t, err, "dial tcp") // error will anyway exist, but it will be not related to config + }) + } +} + +func TestUrlValidation(t *testing.T) { + t.Parallel() + testCases := []struct { + URL string + shouldFail bool + errorString string + }{ + {"", true, "required"}, + {"http://test.url", false, ""}, + {"http://test.url:3999", false, ""}, + {"https://test.url:3999", false, ""}, + {"ftp://test.url", true, "http"}, + {"test.url", true, "http"}, + {"http://127.0.0.1", false, ""}, + {"http://127.0.0.1:3999", false, ""}, + {"test", true, "http"}, + } + + for _, tc := range testCases { + t.Run(tc.URL, func(t *testing.T) { + t.Parallel() + // given + _, err := NewClient(&ClientConfig{ + URL: tc.URL, + APIKey: "test-key", + }) + + // then + if tc.shouldFail { + require.ErrorContains(t, err, "validation failed") + require.ErrorContains(t, err, tc.errorString) + return + } + require.ErrorContains(t, err, "dial tcp") // error will anyway exist, but it will be not related to config + }) + } +} diff --git a/unifi/validation.go b/unifi/validation.go new file mode 100644 index 0000000..29c9546 --- /dev/null +++ b/unifi/validation.go @@ -0,0 +1,77 @@ +package unifi + +import ( + "errors" + "fmt" + + "github.com/go-playground/locales/en" + ut "github.com/go-playground/universal-translator" + vd "github.com/go-playground/validator/v10" + en_translations "github.com/go-playground/validator/v10/translations/en" +) + +// ValidationError is a custom error type for validation errors. +type ValidationError struct { + Root error + Messages map[string]string +} + +// Error returns the error message with combined all validation error messages. +func (v *ValidationError) Error() string { + err := "validation failed: \n" + for field, message := range v.Messages { + err += fmt.Sprintf("%s: %s\n", field, message) + } + return err +} + +// Validator is the interface for the validator. Use it to validate structs. You can register structure-level validations +// with RegisterStructValidation. +type Validator interface { + // Validate validates the given struct and returns an error if the struct is not valid. + Validate(i interface{}) error + // RegisterStructValidation registers a structure-level validation function for a given struct type. + RegisterStructValidation(fn vd.StructLevelFunc, i interface{}) + // RegisterTranslation registers a custom translation for a given tag. + RegisterTranslation(tag string, registerFn vd.RegisterTranslationsFunc, translationFn vd.TranslationFunc) (err error) +} + +type validator struct { + validate *vd.Validate + trans ut.Translator +} + +func (v *validator) Validate(i interface{}) error { + if err := v.validate.Struct(i); err != nil { + var errs vd.ValidationErrors + errors.As(err, &errs) + messages := errs.Translate(v.trans) + + return &ValidationError{Root: err, Messages: messages} + } + return nil +} + +func (v *validator) RegisterStructValidation(f vd.StructLevelFunc, s interface{}) { + v.validate.RegisterStructValidation(f, s) +} + +func (v *validator) RegisterTranslation(tag string, registerFn vd.RegisterTranslationsFunc, translationFn vd.TranslationFunc) error { + return v.validate.RegisterTranslation(tag, v.trans, registerFn, translationFn) +} + +func newValidator() (*validator, error) { + validate := vd.New(vd.WithRequiredStructEnabled()) + enLocale := en.New() + uni := ut.New(enLocale, enLocale) + trans, _ := uni.GetTranslator(enLocale.Locale()) + err := en_translations.RegisterDefaultTranslations(validate, trans) + if err != nil { + return nil, err + } + + return &validator{ + validate: validate, + trans: trans, + }, nil +}