From f198c0f2aaa01d3b04f2f63c3d7f966d64540b67 Mon Sep 17 00:00:00 2001 From: githubsands Date: Mon, 11 Mar 2019 15:50:49 -0600 Subject: [PATCH 1/4] caduceus: remove the reuse of "caduceusHandler" as both a field of the ServerHandler and the authorizationHandler - rename to PrimaryHandler. --- src/caduceus/caduceus.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/caduceus/caduceus.go b/src/caduceus/caduceus.go index 1e42506e..26c888f3 100644 --- a/src/caduceus/caduceus.go +++ b/src/caduceus/caduceus.go @@ -19,8 +19,6 @@ package main import ( "crypto/tls" "fmt" - "github.com/Comcast/webpa-common/service/servicecfg" - "github.com/go-kit/kit/log/level" "net/http" _ "net/http/pprof" "net/url" @@ -28,6 +26,9 @@ import ( "os/signal" "time" + "github.com/Comcast/webpa-common/service/servicecfg" + "github.com/go-kit/kit/log/level" + "github.com/Comcast/webpa-common/concurrent" "github.com/Comcast/webpa-common/logging" "github.com/Comcast/webpa-common/secure" @@ -176,11 +177,11 @@ func caduceus(arguments []string) int { Logger: logger, } - caduceusHandler := alice.New(authHandler.Decorate) + primaryHandler := alice.New(authHandler.Decorate) router := mux.NewRouter() - router = configServerRouter(router, caduceusHandler, serverWrapper) + router = configServerRouter(router, primaryHandler, serverWrapper) webhookFactory, err := webhook.NewFactory(v) if err != nil { @@ -191,8 +192,8 @@ func caduceus(arguments []string) int { webhookFactory.SetExternalUpdate(caduceusSenderWrapper.Update) // register webhook end points for api - router.Handle("/hook", caduceusHandler.ThenFunc(webhookRegistry.UpdateRegistry)) - router.Handle("/hooks", caduceusHandler.ThenFunc(webhookRegistry.GetRegistry)) + router.Handle("/hook", primaryHandler.ThenFunc(webhookRegistry.UpdateRegistry)) + router.Handle("/hooks", primaryHandler.ThenFunc(webhookRegistry.GetRegistry)) scheme := v.GetString("scheme") if len(scheme) < 1 { @@ -287,12 +288,12 @@ func caduceus(arguments []string) int { return 0 } -func configServerRouter(router *mux.Router, caduceusHandler alice.Chain, serverWrapper *ServerHandler) *mux.Router { +func configServerRouter(router *mux.Router, primaryHandler alice.Chain, serverWrapper *ServerHandler) *mux.Router { var singleContentType = func(r *http.Request, _ *mux.RouteMatch) bool { return len(r.Header["Content-Type"]) == 1 //require single specification for Content-Type Header } - router.Handle("/api/v3/notify", caduceusHandler.Then(serverWrapper)).Methods("POST"). + router.Handle("/api/v3/notify", primaryHandler.Then(serverWrapper)).Methods("POST"). HeadersRegexp("Content-Type", "application/msgpack").MatcherFunc(singleContentType) return router From 617dd58b190bb1d5b358c96c305e143af47d85b2 Mon Sep 17 00:00:00 2001 From: githubsands Date: Mon, 11 Mar 2019 16:55:06 -0600 Subject: [PATCH 2/4] caduceus: create primaryHandler, a router that handles all the primary route path (1) authorizating a request and then (2) notification "notify" --- src/caduceus/caduceus.go | 82 ++-------------------- src/caduceus/caduceus_test.go | 85 ----------------------- src/caduceus/caduceus_type.go | 14 +--- src/caduceus/primaryHandler.go | 102 ++++++++++++++++++++++++++++ src/caduceus/primaryHandler_test.go | 91 +++++++++++++++++++++++++ 5 files changed, 200 insertions(+), 174 deletions(-) create mode 100644 src/caduceus/primaryHandler.go create mode 100644 src/caduceus/primaryHandler_test.go diff --git a/src/caduceus/caduceus.go b/src/caduceus/caduceus.go index 26c888f3..ad5b68ac 100644 --- a/src/caduceus/caduceus.go +++ b/src/caduceus/caduceus.go @@ -31,15 +31,9 @@ import ( "github.com/Comcast/webpa-common/concurrent" "github.com/Comcast/webpa-common/logging" - "github.com/Comcast/webpa-common/secure" - "github.com/Comcast/webpa-common/secure/handler" - "github.com/Comcast/webpa-common/secure/key" "github.com/Comcast/webpa-common/server" "github.com/Comcast/webpa-common/webhook" "github.com/Comcast/webpa-common/webhook/aws" - "github.com/SermoDigital/jose/jwt" - "github.com/gorilla/mux" - "github.com/justinas/alice" "github.com/spf13/pflag" "github.com/spf13/viper" ) @@ -49,48 +43,6 @@ const ( DEFAULT_KEY_ID = "current" ) -// getValidator returns validator for JWT tokens -func getValidator(v *viper.Viper) (validator secure.Validator, err error) { - var jwtVals []JWTValidator - - v.UnmarshalKey("jwtValidators", &jwtVals) - - // if a JWTKeys section was supplied, configure a JWS validator - // and append it to the chain of validators - validators := make(secure.Validators, 0, len(jwtVals)) - - for _, validatorDescriptor := range jwtVals { - var keyResolver key.Resolver - keyResolver, err = validatorDescriptor.Keys.NewResolver() - if err != nil { - validator = validators - return - } - - validators = append( - validators, - secure.JWSValidator{ - DefaultKeyId: DEFAULT_KEY_ID, - Resolver: keyResolver, - JWTValidators: []*jwt.Validator{validatorDescriptor.Custom.New()}, - }, - ) - } - - // TODO: This should really be part of the unmarshalled validators somehow - basicAuth := v.GetStringSlice("authHeader") - for _, authValue := range basicAuth { - validators = append( - validators, - secure.ExactMatchValidator(authValue), - ) - } - - validator = validators - - return -} - // caduceus is the driver function for Caduceus. It performs everything main() would do, // except for obtaining the command-line arguments (which are passed to it). @@ -164,25 +116,12 @@ func caduceus(arguments []string) int { maxOutstanding: 0, } - validator, err := getValidator(v) + primaryHandler, err := NewPrimaryHandler(logger, v, serverWrapper) if err != nil { fmt.Fprintf(os.Stderr, "Validator error: %v\n", err) return 1 } - authHandler := handler.AuthorizationHandler{ - HeaderName: "Authorization", - ForbiddenStatusCode: 403, - Validator: validator, - Logger: logger, - } - - primaryHandler := alice.New(authHandler.Decorate) - - router := mux.NewRouter() - - router = configServerRouter(router, primaryHandler, serverWrapper) - webhookFactory, err := webhook.NewFactory(v) if err != nil { fmt.Fprintf(os.Stderr, "Error creating new webhook factory: %s\n", err) @@ -192,8 +131,8 @@ func caduceus(arguments []string) int { webhookFactory.SetExternalUpdate(caduceusSenderWrapper.Update) // register webhook end points for api - router.Handle("/hook", primaryHandler.ThenFunc(webhookRegistry.UpdateRegistry)) - router.Handle("/hooks", primaryHandler.ThenFunc(webhookRegistry.GetRegistry)) + primaryHandler.HandleFunc("/hook", webhookRegistry.UpdateRegistry) + primaryHandler.HandleFunc("/hooks", webhookRegistry.GetRegistry) scheme := v.GetString("scheme") if len(scheme) < 1 { @@ -205,9 +144,9 @@ func caduceus(arguments []string) int { Host: v.GetString("fqdn") + v.GetString("primary.address"), } - webhookFactory.Initialize(router, selfURL, v.GetString("soa.provider"), webhookHandler, logger, metricsRegistry, nil) + webhookFactory.Initialize(primaryHandler, selfURL, v.GetString("soa.provider"), webhookHandler, logger, metricsRegistry, nil) - _, runnable, done := webPA.Prepare(logger, nil, metricsRegistry, router) + _, runnable, done := webPA.Prepare(logger, nil, metricsRegistry, primaryHandler) waitGroup, shutdown, err := concurrent.Execute(runnable) if err != nil { @@ -288,17 +227,6 @@ func caduceus(arguments []string) int { return 0 } -func configServerRouter(router *mux.Router, primaryHandler alice.Chain, serverWrapper *ServerHandler) *mux.Router { - var singleContentType = func(r *http.Request, _ *mux.RouteMatch) bool { - return len(r.Header["Content-Type"]) == 1 //require single specification for Content-Type Header - } - - router.Handle("/api/v3/notify", primaryHandler.Then(serverWrapper)).Methods("POST"). - HeadersRegexp("Content-Type", "application/msgpack").MatcherFunc(singleContentType) - - return router -} - func main() { os.Exit(caduceus(os.Args)) } diff --git a/src/caduceus/caduceus_test.go b/src/caduceus/caduceus_test.go index cbb0e841..831ade23 100644 --- a/src/caduceus/caduceus_test.go +++ b/src/caduceus/caduceus_test.go @@ -17,16 +17,6 @@ package main import ( - "github.com/Comcast/webpa-common/logging" - "github.com/Comcast/webpa-common/secure" - "github.com/Comcast/webpa-common/secure/handler" - "github.com/gorilla/mux" - "github.com/justinas/alice" - "github.com/spf13/viper" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" - "net/http" - "net/http/httptest" "os" "testing" ) @@ -34,78 +24,3 @@ import ( func TestMain(m *testing.M) { os.Exit(m.Run()) } - -/* -Simply tests that no bad requests make it to the caduceus listener. -*/ - -func TestMuxServerConfig(t *testing.T) { - assert := assert.New(t) - - logger := logging.DefaultLogger() - fakeHandler := new(mockHandler) - fakeHandler.On("HandleRequest", mock.AnythingOfType("int"), - mock.AnythingOfType("*wrp.Message")).Return().Once() - - fakeEmptyRequests := new(mockCounter) - fakeErrorRequests := new(mockCounter) - fakeInvalidCount := new(mockCounter) - - fakeQueueDepth := new(mockGauge) - fakeQueueDepth.On("Add", mock.AnythingOfType("float64")).Return().Times(2) - - serverWrapper := &ServerHandler{ - Logger: logger, - caduceusHandler: fakeHandler, - errorRequests: fakeErrorRequests, - emptyRequests: fakeEmptyRequests, - incomingQueueDepthMetric: fakeQueueDepth, - invalidCount: fakeInvalidCount, - } - - authHandler := handler.AuthorizationHandler{Validator: nil} - caduceusHandler := alice.New(authHandler.Decorate) - router := configServerRouter(mux.NewRouter(), caduceusHandler, serverWrapper) - - t.Run("TestMuxResponseCorrectMSP", func(t *testing.T) { - req := exampleRequest("1234", "application/msgpack", "/api/v3/notify") - - req.Header.Set("Content-Type", "application/msgpack") - w := httptest.NewRecorder() - - router.ServeHTTP(w, req) - resp := w.Result() - - assert.Equal(http.StatusAccepted, resp.StatusCode) - }) -} - -func TestGetValidator(t *testing.T) { - assert := assert.New(t) - - fakeViper := viper.New() - - t.Run("TestAuthHeaderNotSet", func(t *testing.T) { - validator, err := getValidator(fakeViper) - - assert.Nil(err) - - validators := validator.(secure.Validators) - assert.Equal(0, len(validators)) - }) - - t.Run("TestAuthHeaderSet", func(t *testing.T) { - expectedAuthHeader := []string{"Basic xxxxxxx"} - fakeViper.Set("authHeader", expectedAuthHeader) - - validator, err := getValidator(fakeViper) - - assert.Nil(err) - - validators := validator.(secure.Validators) - assert.Equal(1, len(validators)) - - exactMatchValidator := validators[0].(secure.ExactMatchValidator) - assert.Equal(expectedAuthHeader[0], string(exactMatchValidator)) - }) -} diff --git a/src/caduceus/caduceus_type.go b/src/caduceus/caduceus_type.go index 67189ba7..4d6ad674 100644 --- a/src/caduceus/caduceus_type.go +++ b/src/caduceus/caduceus_type.go @@ -17,13 +17,12 @@ package main import ( + "time" + "github.com/Comcast/webpa-common/logging" - "github.com/Comcast/webpa-common/secure" - "github.com/Comcast/webpa-common/secure/key" "github.com/Comcast/webpa-common/wrp" "github.com/go-kit/kit/log" "github.com/go-kit/kit/metrics" - "time" ) // Below is the struct we're using to contain the data from a provided config file @@ -48,15 +47,6 @@ type SenderConfig struct { DeliveryInterval time.Duration } -type JWTValidator struct { - // JWTKeys is used to create the key.Resolver for JWT verification keys - Keys key.ResolverFactory - - // Custom is an optional configuration section that defines - // custom rules for validation over and above the standard RFC rules. - Custom secure.JWTValidatorFactory -} - type CaduceusMetricsRegistry interface { NewCounter(name string) metrics.Counter NewGauge(name string) metrics.Gauge diff --git a/src/caduceus/primaryHandler.go b/src/caduceus/primaryHandler.go new file mode 100644 index 00000000..52ab292f --- /dev/null +++ b/src/caduceus/primaryHandler.go @@ -0,0 +1,102 @@ +package main + +import ( + "fmt" + "net/http" + + "github.com/Comcast/webpa-common/secure" + "github.com/Comcast/webpa-common/secure/handler" + "github.com/Comcast/webpa-common/secure/key" + "github.com/SermoDigital/jose/jwt" + "github.com/go-kit/kit/log" + "github.com/gorilla/mux" + "github.com/justinas/alice" + "github.com/spf13/viper" +) + +const ( + baseURI = "api" + version = "v3" +) + +type JWTValidator struct { + // JWTKeys is used to create the key.Resolver for JWT verification keys + Keys key.ResolverFactory + + // Custom is an optional configuration section that defines + // custom rules for validation over and above the standard RFC rules. + Custom secure.JWTValidatorFactory +} + +func NewPrimaryHandler(l log.Logger, v *viper.Viper, sw *ServerHandler) (*mux.Router, error) { + var ( + router = mux.NewRouter() + ) + + validator, err := getValidator(v) + if err != nil { + return nil, err + } + + authHandler := handler.AuthorizationHandler{ + HeaderName: "Authorization", + ForbiddenStatusCode: 403, + Validator: validator, + Logger: l, + } + + authorizationDecorator := alice.New(authHandler.Decorate) + + return configServerRouter(router, authorizationDecorator, sw), nil +} + +func configServerRouter(router *mux.Router, primaryHandler alice.Chain, serverWrapper *ServerHandler) *mux.Router { + var singleContentType = func(r *http.Request, _ *mux.RouteMatch) bool { + return len(r.Header["Content-Type"]) == 1 //require single specification for Content-Type Header + } + + router.Handle("/"+fmt.Sprintf("%s/%s", baseURI, version)+"/notify", primaryHandler.Then(serverWrapper)).Methods("POST").HeadersRegexp("Content-Type", "application/msgpack").MatcherFunc(singleContentType) + + return router +} + +func getValidator(v *viper.Viper) (validator secure.Validator, err error) { + var jwtVals []JWTValidator + + v.UnmarshalKey("jwtValidators", &jwtVals) + + // if a JWTKeys section was supplied, configure a JWS validator + // and append it to the chain of validators + validators := make(secure.Validators, 0, len(jwtVals)) + + for _, validatorDescriptor := range jwtVals { + var keyResolver key.Resolver + keyResolver, err = validatorDescriptor.Keys.NewResolver() + if err != nil { + validator = validators + return + } + + validators = append( + validators, + secure.JWSValidator{ + DefaultKeyId: DEFAULT_KEY_ID, + Resolver: keyResolver, + JWTValidators: []*jwt.Validator{validatorDescriptor.Custom.New()}, + }, + ) + } + + // TODO: This should really be part of the unmarshalled validators somehow + basicAuth := v.GetStringSlice("authHeader") + for _, authValue := range basicAuth { + validators = append( + validators, + secure.ExactMatchValidator(authValue), + ) + } + + validator = validators + + return +} diff --git a/src/caduceus/primaryHandler_test.go b/src/caduceus/primaryHandler_test.go new file mode 100644 index 00000000..974cd72f --- /dev/null +++ b/src/caduceus/primaryHandler_test.go @@ -0,0 +1,91 @@ +package main + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/Comcast/webpa-common/logging" + "github.com/Comcast/webpa-common/secure" + "github.com/Comcast/webpa-common/secure/handler" + "github.com/gorilla/mux" + "github.com/justinas/alice" + "github.com/spf13/viper" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" +) + +func TestGetValidator(t *testing.T) { + assert := assert.New(t) + + fakeViper := viper.New() + + t.Run("TestAuthHeaderNotSet", func(t *testing.T) { + validator, err := getValidator(fakeViper) + + assert.Nil(err) + + validators := validator.(secure.Validators) + assert.Equal(0, len(validators)) + }) + + t.Run("TestAuthHeaderSet", func(t *testing.T) { + expectedAuthHeader := []string{"Basic xxxxxxx"} + fakeViper.Set("authHeader", expectedAuthHeader) + + validator, err := getValidator(fakeViper) + + assert.Nil(err) + + validators := validator.(secure.Validators) + assert.Equal(1, len(validators)) + + exactMatchValidator := validators[0].(secure.ExactMatchValidator) + assert.Equal(expectedAuthHeader[0], string(exactMatchValidator)) + }) +} + +/* +Simply tests that no bad requests make it to the caduceus listener. +*/ +func TestMuxServerConfig(t *testing.T) { + assert := assert.New(t) + + logger := logging.DefaultLogger() + fakeHandler := new(mockHandler) + fakeHandler.On("HandleRequest", mock.AnythingOfType("int"), + mock.AnythingOfType("*wrp.Message")).Return().Once() + + fakeEmptyRequests := new(mockCounter) + fakeErrorRequests := new(mockCounter) + fakeInvalidCount := new(mockCounter) + + fakeQueueDepth := new(mockGauge) + fakeQueueDepth.On("Add", mock.AnythingOfType("float64")).Return().Times(2) + + serverWrapper := &ServerHandler{ + Logger: logger, + caduceusHandler: fakeHandler, + errorRequests: fakeErrorRequests, + emptyRequests: fakeEmptyRequests, + incomingQueueDepthMetric: fakeQueueDepth, + invalidCount: fakeInvalidCount, + } + + authHandler := handler.AuthorizationHandler{Validator: nil} + caduceusHandler := alice.New(authHandler.Decorate) + + router := configServerRouter(mux.NewRouter(), caduceusHandler, serverWrapper) + + t.Run("TestMuxResponseCorrectMSP", func(t *testing.T) { + req := exampleRequest("1234", "application/msgpack", "/api/v3/notify") + + req.Header.Set("Content-Type", "application/msgpack") + w := httptest.NewRecorder() + + router.ServeHTTP(w, req) + resp := w.Result() + + assert.Equal(http.StatusAccepted, resp.StatusCode) + }) +} From 73afe01e40102554ae24acd1ae871a3ff121ab97 Mon Sep 17 00:00:00 2001 From: githubsands Date: Tue, 12 Mar 2019 13:16:50 -0600 Subject: [PATCH 3/4] caduceus: test NewPrimaryHandler so code coverage passes --- src/caduceus/primaryHandler_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/caduceus/primaryHandler_test.go b/src/caduceus/primaryHandler_test.go index 974cd72f..6b50dae3 100644 --- a/src/caduceus/primaryHandler_test.go +++ b/src/caduceus/primaryHandler_test.go @@ -15,6 +15,18 @@ import ( "github.com/stretchr/testify/mock" ) +func TestNewPrimaryHandler(t *testing.T) { + var ( + l = logging.New(nil) + viper = viper.New() + sw = &ServerHandler{} + ) + + if _, err := NewPrimaryHandler(l, viper, sw); err != nil { + t.Fatalf("NewPrimaryHandler failed: %v", err) + } +} + func TestGetValidator(t *testing.T) { assert := assert.New(t) From e957bfafe58e3d1a60a00220150d86125be06552 Mon Sep 17 00:00:00 2001 From: githubsands Date: Tue, 12 Mar 2019 16:43:27 -0600 Subject: [PATCH 4/4] caduceus: increase test coverage --- src/caduceus/primaryHandler_test.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/caduceus/primaryHandler_test.go b/src/caduceus/primaryHandler_test.go index 6b50dae3..0d201a24 100644 --- a/src/caduceus/primaryHandler_test.go +++ b/src/caduceus/primaryHandler_test.go @@ -17,14 +17,17 @@ import ( func TestNewPrimaryHandler(t *testing.T) { var ( - l = logging.New(nil) - viper = viper.New() - sw = &ServerHandler{} + l = logging.New(nil) + viper = viper.New() + sw = &ServerHandler{} + expectedAuthHeader = []string{"Basic xxxxxxx"} ) + viper.Set("authHeader", expectedAuthHeader) if _, err := NewPrimaryHandler(l, viper, sw); err != nil { t.Fatalf("NewPrimaryHandler failed: %v", err) } + } func TestGetValidator(t *testing.T) {