From 40c53ea0d908bf5481bb8cbeb97e3c97a6e17d40 Mon Sep 17 00:00:00 2001 From: ErickSkrauch Date: Sun, 16 Feb 2020 13:23:47 +0300 Subject: [PATCH] Add stats reporter events listener, restore all events for http layer, rework authentication middleware and authenticator interface --- auth/jwt_test.go | 97 ------------- cmd/serve.go | 3 +- cmd/token.go | 6 +- dispatcher/dispatcher.go | 12 +- eventsubscribers/stats_reporter.go | 85 +++++++++++ eventsubscribers/stats_reporter_test.go | 178 ++++++++++++++++++++++++ http/http.go | 50 +++++++ http/http_test.go | 80 ++++++++++- {auth => http}/jwt.go | 30 ++-- http/jwt_test.go | 127 +++++++++++++++++ http/skinsystem.go | 53 ++----- http/skinsystem_test.go | 66 ++++++--- 12 files changed, 602 insertions(+), 185 deletions(-) delete mode 100644 auth/jwt_test.go create mode 100644 eventsubscribers/stats_reporter.go create mode 100644 eventsubscribers/stats_reporter_test.go rename {auth => http}/jwt.go (63%) create mode 100644 http/jwt_test.go diff --git a/auth/jwt_test.go b/auth/jwt_test.go deleted file mode 100644 index 00fc65a..0000000 --- a/auth/jwt_test.go +++ /dev/null @@ -1,97 +0,0 @@ -package auth - -import ( - "net/http/httptest" - "testing" - - testify "github.com/stretchr/testify/assert" -) - -const jwt = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxNTE2NjU4MTkzIiwic2NvcGVzIjoic2tpbiJ9.agbBS0qdyYMBaVfTZJAZcTTRgW1Y0kZty4H3N2JHBO8" - -func TestJwtAuth_NewToken_Success(t *testing.T) { - assert := testify.New(t) - - jwt := &JwtAuth{[]byte("secret")} - token, err := jwt.NewToken(SkinScope) - assert.Nil(err) - assert.NotNil(token) -} - -func TestJwtAuth_NewToken_KeyNotAvailable(t *testing.T) { - assert := testify.New(t) - - jwt := &JwtAuth{} - token, err := jwt.NewToken(SkinScope) - assert.Error(err, "signing key not available") - assert.Nil(token) -} - -func TestJwtAuth_Check_EmptyRequest(t *testing.T) { - assert := testify.New(t) - - req := httptest.NewRequest("POST", "http://localhost", nil) - jwt := &JwtAuth{[]byte("secret")} - - err := jwt.Check(req) - assert.IsType(&Unauthorized{}, err) - assert.EqualError(err, "Authentication header not presented") -} - -func TestJwtAuth_Check_NonBearer(t *testing.T) { - assert := testify.New(t) - - req := httptest.NewRequest("POST", "http://localhost", nil) - req.Header.Add("Authorization", "this is not jwt") - jwt := &JwtAuth{[]byte("secret")} - - err := jwt.Check(req) - assert.IsType(&Unauthorized{}, err) - assert.EqualError(err, "Cannot recognize JWT token in passed value") -} - -func TestJwtAuth_Check_BearerButNotJwt(t *testing.T) { - assert := testify.New(t) - - req := httptest.NewRequest("POST", "http://localhost", nil) - req.Header.Add("Authorization", "Bearer thisIs.Not.Jwt") - jwt := &JwtAuth{[]byte("secret")} - - err := jwt.Check(req) - assert.IsType(&Unauthorized{}, err) - assert.EqualError(err, "Cannot parse passed JWT token") -} - -func TestJwtAuth_Check_SecretNotAvailable(t *testing.T) { - assert := testify.New(t) - - req := httptest.NewRequest("POST", "http://localhost", nil) - req.Header.Add("Authorization", "Bearer " + jwt) - jwt := &JwtAuth{} - - err := jwt.Check(req) - assert.Error(err, "Signing key not set") -} - -func TestJwtAuth_Check_SecretInvalid(t *testing.T) { - assert := testify.New(t) - - req := httptest.NewRequest("POST", "http://localhost", nil) - req.Header.Add("Authorization", "Bearer " + jwt) - jwt := &JwtAuth{[]byte("this is another secret")} - - err := jwt.Check(req) - assert.IsType(&Unauthorized{}, err) - assert.EqualError(err, "JWT token have invalid signature. It may be corrupted or expired.") -} - -func TestJwtAuth_Check_Valid(t *testing.T) { - assert := testify.New(t) - - req := httptest.NewRequest("POST", "http://localhost", nil) - req.Header.Add("Authorization", "Bearer " + jwt) - jwt := &JwtAuth{[]byte("secret")} - - err := jwt.Check(req) - assert.Nil(err) -} diff --git a/cmd/serve.go b/cmd/serve.go index 5d96bae..31d0680 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -8,7 +8,6 @@ import ( "github.com/spf13/cobra" "github.com/spf13/viper" - "github.com/elyby/chrly/auth" "github.com/elyby/chrly/bootstrap" "github.com/elyby/chrly/db" "github.com/elyby/chrly/http" @@ -82,7 +81,7 @@ var serveCmd = &cobra.Command{ SkinsRepo: skinsRepo, CapesRepo: capesRepo, MojangTexturesProvider: mojangTexturesProvider, - Auth: &auth.JwtAuth{Key: []byte(viper.GetString("chrly.secret"))}, + Authenticator: &http.JwtAuth{Key: []byte(viper.GetString("chrly.secret"))}, TexturesExtraParamName: viper.GetString("textures.extra_param_name"), TexturesExtraParamValue: viper.GetString("textures.extra_param_value"), }).CreateHandler() diff --git a/cmd/token.go b/cmd/token.go index 2380d36..ba891fb 100644 --- a/cmd/token.go +++ b/cmd/token.go @@ -4,7 +4,7 @@ import ( "fmt" "log" - "github.com/elyby/chrly/auth" + "github.com/elyby/chrly/http" "github.com/spf13/cobra" "github.com/spf13/viper" @@ -14,8 +14,8 @@ var tokenCmd = &cobra.Command{ Use: "token", Short: "Creates a new token, which allows to interact with Chrly API", Run: func(cmd *cobra.Command, args []string) { - jwtAuth := &auth.JwtAuth{Key: []byte(viper.GetString("chrly.secret"))} - token, err := jwtAuth.NewToken(auth.SkinScope) + jwtAuth := &http.JwtAuth{Key: []byte(viper.GetString("chrly.secret"))} + token, err := jwtAuth.NewToken(http.SkinScope) if err != nil { log.Fatalf("Unable to create new token. The error is %v\n", err) } diff --git a/dispatcher/dispatcher.go b/dispatcher/dispatcher.go index e8d1515..8488202 100644 --- a/dispatcher/dispatcher.go +++ b/dispatcher/dispatcher.go @@ -3,20 +3,20 @@ package dispatcher import "github.com/asaskevich/EventBus" type EventDispatcher interface { - Subscribe(name string, fn interface{}) - Emit(name string, args ...interface{}) + Subscribe(topic string, fn interface{}) + Emit(topic string, args ...interface{}) } type LocalEventDispatcher struct { bus EventBus.Bus } -func (d *LocalEventDispatcher) Subscribe(name string, fn interface{}) { - _ = d.bus.Subscribe(name, fn) +func (d *LocalEventDispatcher) Subscribe(topic string, fn interface{}) { + _ = d.bus.Subscribe(topic, fn) } -func (d *LocalEventDispatcher) Emit(name string, args ...interface{}) { - d.bus.Publish(name, args...) +func (d *LocalEventDispatcher) Emit(topic string, args ...interface{}) { + d.bus.Publish(topic, args...) } func New() EventDispatcher { diff --git a/eventsubscribers/stats_reporter.go b/eventsubscribers/stats_reporter.go new file mode 100644 index 0000000..ad0e449 --- /dev/null +++ b/eventsubscribers/stats_reporter.go @@ -0,0 +1,85 @@ +package eventsubscribers + +import ( + "net/http" + "strings" + + "github.com/mono83/slf" + + "github.com/elyby/chrly/dispatcher" +) + +type StatsReporter struct { + Reporter slf.StatsReporter + Prefix string +} + +func (s *StatsReporter) ConfigureWithDispatcher(d dispatcher.EventDispatcher) { + d.Subscribe("skinsystem:before_request", s.handleBeforeRequest) + d.Subscribe("skinsystem:after_request", s.handleAfterRequest) + + d.Subscribe("authenticator:success", s.incCounterHandler("authentication.challenge")) // TODO: legacy, remove in v5 + d.Subscribe("authenticator:success", s.incCounterHandler("authentication.success")) + d.Subscribe("authentication:error", s.incCounterHandler("authentication.challenge")) // TODO: legacy, remove in v5 + d.Subscribe("authentication:error", s.incCounterHandler("authentication.failed")) +} + +func (s *StatsReporter) handleBeforeRequest(req *http.Request) { + var key string + m := req.Method + p := req.URL.Path + if p == "/skins" { + key = "skins.get_request" + } else if strings.HasPrefix(p, "/skins/") { + key = "skins.request" + } else if p == "/cloaks" { + key = "capes.get_request" + } else if strings.HasPrefix(p, "/cloaks/") { + key = "capes.request" + } else if strings.HasPrefix(p, "/textures/signed/") { + key = "signed_textures.request" + } else if strings.HasPrefix(p, "/textures/") { + key = "textures.request" + } else if m == http.MethodPost && p == "/api/skins" { + key = "api.skins.post.request" + } else if m == http.MethodDelete && strings.HasPrefix(p, "/api/skins/") { + key = "api.skins.delete.request" + } else { + return + } + + s.incCounter(key) +} + +func (s *StatsReporter) handleAfterRequest(req *http.Request, code int) { + var key string + m := req.Method + p := req.URL.Path + if m == http.MethodPost && p == "/api/skins" && code == http.StatusCreated { + key = "api.skins.post.success" + } else if m == http.MethodPost && p == "/api/skins" && code == http.StatusBadRequest { + key = "api.skins.post.validation_failed" + } else if m == http.MethodDelete && strings.HasPrefix(p, "/api/skins/") && code == http.StatusNoContent { + key = "api.skins.delete.success" + } else if m == http.MethodDelete && strings.HasPrefix(p, "/api/skins/") && code == http.StatusNotFound { + key = "api.skins.delete.not_found" + } else { + return + } + + s.incCounter(key) +} + +func (s *StatsReporter) incCounterHandler(name string) func(...interface{}) { + return func(...interface{}) { + s.incCounter(name) + } +} + +func (s *StatsReporter) incCounter(name string) { + s.Reporter.IncCounter(s.key(name), 1) +} + +func (s *StatsReporter) key(name string) string { + return strings.Join([]string{s.Prefix, name}, ".") +} diff --git a/eventsubscribers/stats_reporter_test.go b/eventsubscribers/stats_reporter_test.go new file mode 100644 index 0000000..7d0ea24 --- /dev/null +++ b/eventsubscribers/stats_reporter_test.go @@ -0,0 +1,178 @@ +package eventsubscribers + +import ( + "errors" + "net/http/httptest" + "testing" + + "github.com/elyby/chrly/dispatcher" + "github.com/elyby/chrly/tests" +) + +type StatsReporterTestCase struct { + Topic string + Args []interface{} + ExpectedCalls [][]interface{} +} + +var statsReporterTestCases = []*StatsReporterTestCase{ + // Before request + { + Topic: "skinsystem:before_request", + Args: []interface{}{httptest.NewRequest("GET", "http://localhost/skins/username", nil)}, + ExpectedCalls: [][]interface{}{ + {"IncCounter", "mock_prefix.skins.request", int64(1)}, + }, + }, + { + Topic: "skinsystem:before_request", + Args: []interface{}{httptest.NewRequest("GET", "http://localhost/skins?name=username", nil)}, + ExpectedCalls: [][]interface{}{ + {"IncCounter", "mock_prefix.skins.get_request", int64(1)}, + }, + }, + { + Topic: "skinsystem:before_request", + Args: []interface{}{httptest.NewRequest("GET", "http://localhost/cloaks/username", nil)}, + ExpectedCalls: [][]interface{}{ + {"IncCounter", "mock_prefix.capes.request", int64(1)}, + }, + }, + { + Topic: "skinsystem:before_request", + Args: []interface{}{httptest.NewRequest("GET", "http://localhost/cloaks?name=username", nil)}, + ExpectedCalls: [][]interface{}{ + {"IncCounter", "mock_prefix.capes.get_request", int64(1)}, + }, + }, + { + Topic: "skinsystem:before_request", + Args: []interface{}{httptest.NewRequest("GET", "http://localhost/textures/username", nil)}, + ExpectedCalls: [][]interface{}{ + {"IncCounter", "mock_prefix.textures.request", int64(1)}, + }, + }, + { + Topic: "skinsystem:before_request", + Args: []interface{}{httptest.NewRequest("GET", "http://localhost/textures/signed/username", nil)}, + ExpectedCalls: [][]interface{}{ + {"IncCounter", "mock_prefix.signed_textures.request", int64(1)}, + }, + }, + { + Topic: "skinsystem:before_request", + Args: []interface{}{httptest.NewRequest("POST", "http://localhost/api/skins", nil)}, + ExpectedCalls: [][]interface{}{ + {"IncCounter", "mock_prefix.api.skins.post.request", int64(1)}, + }, + }, + { + Topic: "skinsystem:before_request", + Args: []interface{}{httptest.NewRequest("DELETE", "http://localhost/api/skins/username", nil)}, + ExpectedCalls: [][]interface{}{ + {"IncCounter", "mock_prefix.api.skins.delete.request", int64(1)}, + }, + }, + { + Topic: "skinsystem:before_request", + Args: []interface{}{httptest.NewRequest("DELETE", "http://localhost/api/skins/id:1", nil)}, + ExpectedCalls: [][]interface{}{ + {"IncCounter", "mock_prefix.api.skins.delete.request", int64(1)}, + }, + }, + { + Topic: "skinsystem:before_request", + Args: []interface{}{httptest.NewRequest("GET", "http://localhost/unknown", nil)}, + ExpectedCalls: nil, + }, + // After request + { + Topic: "skinsystem:after_request", + Args: []interface{}{httptest.NewRequest("POST", "http://localhost/api/skins", nil), 201}, + ExpectedCalls: [][]interface{}{ + {"IncCounter", "mock_prefix.api.skins.post.success", int64(1)}, + }, + }, + { + Topic: "skinsystem:after_request", + Args: []interface{}{httptest.NewRequest("POST", "http://localhost/api/skins", nil), 400}, + ExpectedCalls: [][]interface{}{ + {"IncCounter", "mock_prefix.api.skins.post.validation_failed", int64(1)}, + }, + }, + { + Topic: "skinsystem:after_request", + Args: []interface{}{httptest.NewRequest("DELETE", "http://localhost/api/skins/username", nil), 204}, + ExpectedCalls: [][]interface{}{ + {"IncCounter", "mock_prefix.api.skins.delete.success", int64(1)}, + }, + }, + { + Topic: "skinsystem:after_request", + Args: []interface{}{httptest.NewRequest("DELETE", "http://localhost/api/skins/username", nil), 404}, + ExpectedCalls: [][]interface{}{ + {"IncCounter", "mock_prefix.api.skins.delete.not_found", int64(1)}, + }, + }, + { + Topic: "skinsystem:after_request", + Args: []interface{}{httptest.NewRequest("DELETE", "http://localhost/api/skins/id:1", nil), 204}, + ExpectedCalls: [][]interface{}{ + {"IncCounter", "mock_prefix.api.skins.delete.success", int64(1)}, + }, + }, + { + Topic: "skinsystem:after_request", + Args: []interface{}{httptest.NewRequest("DELETE", "http://localhost/api/skins/id:1", nil), 404}, + ExpectedCalls: [][]interface{}{ + {"IncCounter", "mock_prefix.api.skins.delete.not_found", int64(1)}, + }, + }, + { + Topic: "skinsystem:after_request", + Args: []interface{}{httptest.NewRequest("DELETE", "http://localhost/unknown", nil), 404}, + ExpectedCalls: nil, + }, + // Authenticator + { + Topic: "authenticator:success", + Args: []interface{}{}, + ExpectedCalls: [][]interface{}{ + {"IncCounter", "mock_prefix.authentication.challenge", int64(1)}, + {"IncCounter", "mock_prefix.authentication.success", int64(1)}, + }, + }, + { + Topic: "authentication:error", + Args: []interface{}{errors.New("error")}, + ExpectedCalls: [][]interface{}{ + {"IncCounter", "mock_prefix.authentication.challenge", int64(1)}, + {"IncCounter", "mock_prefix.authentication.failed", int64(1)}, + }, + }, +} + +func TestStatsReporter_handleHTTPRequest(t *testing.T) { + for _, c := range statsReporterTestCases { + t.Run(c.Topic, func(t *testing.T) { + wdMock := &tests.WdMock{} + if c.ExpectedCalls != nil { + for _, c := range c.ExpectedCalls { + topicName, _ := c[0].(string) + wdMock.On(topicName, c[1:]...).Once() + } + } + + reporter := &StatsReporter{ + Reporter: wdMock, + Prefix: "mock_prefix", + } + + d := dispatcher.New() + reporter.ConfigureWithDispatcher(d) + d.Emit(c.Topic, c.Args...) + + wdMock.AssertExpectations(t) + }) + } +} diff --git a/http/http.go b/http/http.go index 6a76daf..c1ab843 100644 --- a/http/http.go +++ b/http/http.go @@ -4,7 +4,10 @@ import ( "encoding/json" "net" "net/http" + "strings" "time" + + "github.com/gorilla/mux" ) type Emitter interface { @@ -28,6 +31,53 @@ func Serve(address string, handler http.Handler) error { return server.Serve(listener) } +type loggingResponseWriter struct { + http.ResponseWriter + statusCode int +} + +func (lrw *loggingResponseWriter) WriteHeader(code int) { + lrw.statusCode = code + lrw.ResponseWriter.WriteHeader(code) +} + +func CreateRequestEventsMiddleware(emitter Emitter, prefix string) mux.MiddlewareFunc { + beforeTopic := strings.Join([]string{prefix, "before_request"}, ":") + afterTopic := strings.Join([]string{prefix, "after_request"}, ":") + + return func(handler http.Handler) http.Handler { + return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { + emitter.Emit(beforeTopic, req) + + lrw := &loggingResponseWriter{ + ResponseWriter: resp, + statusCode: http.StatusOK, + } + handler.ServeHTTP(lrw, req) + + emitter.Emit(afterTopic, req, lrw.statusCode) + }) + } +} + +type Authenticator interface { + Authenticate(req *http.Request) error +} + +func CreateAuthenticationMiddleware(checker Authenticator) mux.MiddlewareFunc { + return func(handler http.Handler) http.Handler { + return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { + err := checker.Authenticate(req) + if err != nil { + apiForbidden(resp, err.Error()) + return + } + + handler.ServeHTTP(resp, req) + }) + } +} + func NotFound(response http.ResponseWriter, _ *http.Request) { data, _ := json.Marshal(map[string]string{ "status": "404", diff --git a/http/http_test.go b/http/http_test.go index 376f1f6..d9abfe5 100644 --- a/http/http_test.go +++ b/http/http_test.go @@ -1,7 +1,9 @@ package http import ( + "errors" "io/ioutil" + "net/http" "net/http/httptest" "testing" @@ -14,10 +16,84 @@ type emitterMock struct { } func (e *emitterMock) Emit(name string, args ...interface{}) { - e.Called((append([]interface{}{name}, args...))...) + e.Called(append([]interface{}{name}, args...)...) } -func TestConfig_NotFound(t *testing.T) { +func TestCreateRequestEventsMiddleware(t *testing.T) { + req := httptest.NewRequest("GET", "http://example.com", nil) + resp := httptest.NewRecorder() + + emitter := &emitterMock{} + emitter.On("Emit", "test_prefix:before_request", req) + emitter.On("Emit", "test_prefix:after_request", req, 400) + + isHandlerCalled := false + middlewareFunc := CreateRequestEventsMiddleware(emitter, "test_prefix") + middlewareFunc.Middleware(http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { + resp.WriteHeader(400) + isHandlerCalled = true + })).ServeHTTP(resp, req) + + if !isHandlerCalled { + t.Fatal("Handler isn't called from the middleware") + } + + emitter.AssertExpectations(t) +} + +type authCheckerMock struct { + mock.Mock +} + +func (m *authCheckerMock) Authenticate(req *http.Request) error { + args := m.Called(req) + return args.Error(0) +} + +func TestCreateAuthenticationMiddleware(t *testing.T) { + t.Run("pass", func(t *testing.T) { + req := httptest.NewRequest("GET", "http://example.com", nil) + resp := httptest.NewRecorder() + + auth := &authCheckerMock{} + auth.On("Authenticate", req).Once().Return(nil) + + isHandlerCalled := false + middlewareFunc := CreateAuthenticationMiddleware(auth) + middlewareFunc.Middleware(http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { + isHandlerCalled = true + })).ServeHTTP(resp, req) + + testify.True(t, isHandlerCalled, "Handler isn't called from the middleware") + + auth.AssertExpectations(t) + }) + + t.Run("fail", func(t *testing.T) { + req := httptest.NewRequest("GET", "http://example.com", nil) + resp := httptest.NewRecorder() + + auth := &authCheckerMock{} + auth.On("Authenticate", req).Once().Return(errors.New("error reason")) + + isHandlerCalled := false + middlewareFunc := CreateAuthenticationMiddleware(auth) + middlewareFunc.Middleware(http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { + isHandlerCalled = true + })).ServeHTTP(resp, req) + + testify.False(t, isHandlerCalled, "Handler shouldn't be called") + testify.Equal(t, 403, resp.Code) + body, _ := ioutil.ReadAll(resp.Body) + testify.JSONEq(t, `{ + "error": "error reason" + }`, string(body)) + + auth.AssertExpectations(t) + }) +} + +func TestNotFound(t *testing.T) { assert := testify.New(t) req := httptest.NewRequest("GET", "http://example.com", nil) diff --git a/auth/jwt.go b/http/jwt.go similarity index 63% rename from auth/jwt.go rename to http/jwt.go index 7d86618..94f196b 100644 --- a/auth/jwt.go +++ b/http/jwt.go @@ -1,4 +1,4 @@ -package auth +package http import ( "errors" @@ -21,6 +21,7 @@ var ( ) type JwtAuth struct { + Emitter Key []byte } @@ -41,42 +42,37 @@ func (t *JwtAuth) NewToken(scopes ...Scope) ([]byte, error) { return token, nil } -func (t *JwtAuth) Check(req *http.Request) error { +func (t *JwtAuth) Authenticate(req *http.Request) error { if len(t.Key) == 0 { - return &Unauthorized{"Signing key not set"} + return t.emitErr(errors.New("Signing key not set")) } bearerToken := req.Header.Get("Authorization") if bearerToken == "" { - return &Unauthorized{"Authentication header not presented"} + return t.emitErr(errors.New("Authentication header not presented")) } if !strings.EqualFold(bearerToken[0:7], "BEARER ") { - return &Unauthorized{"Cannot recognize JWT token in passed value"} + return t.emitErr(errors.New("Cannot recognize JWT token in passed value")) } tokenStr := bearerToken[7:] token, err := jws.ParseJWT([]byte(tokenStr)) if err != nil { - return &Unauthorized{"Cannot parse passed JWT token"} + return t.emitErr(errors.New("Cannot parse passed JWT token")) } err = token.Validate(t.Key, hashAlg) if err != nil { - return &Unauthorized{"JWT token have invalid signature. It may be corrupted or expired."} + return t.emitErr(errors.New("JWT token have invalid signature. It may be corrupted or expired")) } + t.Emit("authentication:success") + return nil } -type Unauthorized struct { - Reason string -} - -func (e *Unauthorized) Error() string { - if e.Reason != "" { - return e.Reason - } - - return "Unauthorized" +func (t *JwtAuth) emitErr(err error) error { + t.Emit("authentication:error", err) + return err } diff --git a/http/jwt_test.go b/http/jwt_test.go new file mode 100644 index 0000000..92c9846 --- /dev/null +++ b/http/jwt_test.go @@ -0,0 +1,127 @@ +package http + +import ( + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" +) + +const jwt = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxNTE2NjU4MTkzIiwic2NvcGVzIjoic2tpbiJ9.agbBS0qdyYMBaVfTZJAZcTTRgW1Y0kZty4H3N2JHBO8" + +func TestJwtAuth_NewToken(t *testing.T) { + t.Run("success", func(t *testing.T) { + jwt := &JwtAuth{Key: []byte("secret")} + token, err := jwt.NewToken(SkinScope) + assert.Nil(t, err) + assert.NotNil(t, token) + }) + + t.Run("key not provided", func(t *testing.T) { + jwt := &JwtAuth{} + token, err := jwt.NewToken(SkinScope) + assert.Error(t, err, "signing key not available") + assert.Nil(t, token) + }) +} + +func TestJwtAuth_Authenticate(t *testing.T) { + t.Run("success", func(t *testing.T) { + emitter := &emitterMock{} + emitter.On("Emit", "authentication:success") + + req := httptest.NewRequest("POST", "http://localhost", nil) + req.Header.Add("Authorization", "Bearer " + jwt) + jwt := &JwtAuth{Key: []byte("secret"), Emitter: emitter} + + err := jwt.Authenticate(req) + assert.Nil(t, err) + + emitter.AssertExpectations(t) + }) + + t.Run("request without auth header", func(t *testing.T) { + emitter := &emitterMock{} + emitter.On("Emit", "authentication:error", mock.MatchedBy(func(err error) bool { + assert.Error(t, err, "Authentication header not presented") + return true + })) + + req := httptest.NewRequest("POST", "http://localhost", nil) + jwt := &JwtAuth{Key: []byte("secret"), Emitter: emitter} + + err := jwt.Authenticate(req) + assert.Error(t, err, "Authentication header not presented") + + emitter.AssertExpectations(t) + }) + + t.Run("no bearer token prefix", func(t *testing.T) { + emitter := &emitterMock{} + emitter.On("Emit", "authentication:error", mock.MatchedBy(func(err error) bool { + assert.Error(t, err, "Cannot recognize JWT token in passed value") + return true + })) + + req := httptest.NewRequest("POST", "http://localhost", nil) + req.Header.Add("Authorization", "this is not jwt") + jwt := &JwtAuth{Key: []byte("secret"), Emitter: emitter} + + err := jwt.Authenticate(req) + assert.Error(t, err, "Cannot recognize JWT token in passed value") + + emitter.AssertExpectations(t) + }) + + t.Run("bearer token but not jwt", func(t *testing.T) { + emitter := &emitterMock{} + emitter.On("Emit", "authentication:error", mock.MatchedBy(func(err error) bool { + assert.Error(t, err, "Cannot parse passed JWT token") + return true + })) + + req := httptest.NewRequest("POST", "http://localhost", nil) + req.Header.Add("Authorization", "Bearer thisIs.Not.Jwt") + jwt := &JwtAuth{Key: []byte("secret"), Emitter: emitter} + + err := jwt.Authenticate(req) + assert.Error(t, err, "Cannot parse passed JWT token") + + emitter.AssertExpectations(t) + }) + + t.Run("when secret is not set", func(t *testing.T) { + emitter := &emitterMock{} + emitter.On("Emit", "authentication:error", mock.MatchedBy(func(err error) bool { + assert.Error(t, err, "Signing key not set") + return true + })) + + req := httptest.NewRequest("POST", "http://localhost", nil) + req.Header.Add("Authorization", "Bearer " + jwt) + jwt := &JwtAuth{Emitter: emitter} + + err := jwt.Authenticate(req) + assert.Error(t, err, "Signing key not set") + + emitter.AssertExpectations(t) + }) + + t.Run("invalid signature", func(t *testing.T) { + emitter := &emitterMock{} + emitter.On("Emit", "authentication:error", mock.MatchedBy(func(err error) bool { + assert.Error(t, err, "JWT token have invalid signature. It may be corrupted or expired") + return true + })) + + req := httptest.NewRequest("POST", "http://localhost", nil) + req.Header.Add("Authorization", "Bearer " + jwt) + jwt := &JwtAuth{Key: []byte("this is another secret"), Emitter: emitter} + + err := jwt.Authenticate(req) + assert.Error(t, err, "JWT token have invalid signature. It may be corrupted or expired") + + emitter.AssertExpectations(t) + }) +} diff --git a/http/skinsystem.go b/http/skinsystem.go index f608276..a5c6f11 100644 --- a/http/skinsystem.go +++ b/http/skinsystem.go @@ -14,10 +14,10 @@ import ( "github.com/thedevsaddam/govalidator" "github.com/elyby/chrly/api/mojang" - "github.com/elyby/chrly/auth" "github.com/elyby/chrly/model" ) +//noinspection GoSnakeCaseUsage const UUID_ANY = "^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$" var regexUuidAny = regexp.MustCompile(UUID_ANY) @@ -78,10 +78,6 @@ type MojangTexturesProvider interface { GetForUsername(username string) (*mojang.SignedTexturesResponse, error) } -type AuthChecker interface { - Check(req *http.Request) error -} - type Skinsystem struct { Emitter TexturesExtraParamName string @@ -89,25 +85,26 @@ type Skinsystem struct { SkinsRepo SkinsRepository CapesRepo CapesRepository MojangTexturesProvider MojangTexturesProvider - Auth AuthChecker + Authenticator Authenticator } func (ctx *Skinsystem) CreateHandler() *mux.Router { router := mux.NewRouter().StrictSlash(true) + router.Use(CreateRequestEventsMiddleware(ctx.Emitter, "skinsystem")) - router.HandleFunc("/skins/{username}", ctx.Skin).Methods("GET") - router.HandleFunc("/cloaks/{username}", ctx.Cape).Methods("GET").Name("cloaks") - router.HandleFunc("/textures/{username}", ctx.Textures).Methods("GET") - router.HandleFunc("/textures/signed/{username}", ctx.SignedTextures).Methods("GET") + router.HandleFunc("/skins/{username}", ctx.Skin).Methods(http.MethodGet) + router.HandleFunc("/cloaks/{username}", ctx.Cape).Methods(http.MethodGet).Name("cloaks") + router.HandleFunc("/textures/{username}", ctx.Textures).Methods(http.MethodGet) + router.HandleFunc("/textures/signed/{username}", ctx.SignedTextures).Methods(http.MethodGet) // Legacy - router.HandleFunc("/skins", ctx.SkinGET).Methods("GET") - router.HandleFunc("/cloaks", ctx.CapeGET).Methods("GET") + router.HandleFunc("/skins", ctx.SkinGET).Methods(http.MethodGet) + router.HandleFunc("/cloaks", ctx.CapeGET).Methods(http.MethodGet) // API apiRouter := router.PathPrefix("/api").Subrouter() - apiRouter.Use(ctx.AuthenticationMiddleware) - apiRouter.HandleFunc("/skins", ctx.PostSkin).Methods("POST") - apiRouter.HandleFunc("/skins/id:{id:[0-9]+}", ctx.DeleteSkinByUserId).Methods("DELETE") - apiRouter.HandleFunc("/skins/{username}", ctx.DeleteSkinByUsername).Methods("DELETE") + apiRouter.Use(CreateAuthenticationMiddleware(ctx.Authenticator)) + apiRouter.HandleFunc("/skins", ctx.PostSkin).Methods(http.MethodPost) + apiRouter.HandleFunc("/skins/id:{id:[0-9]+}", ctx.DeleteSkinByUserId).Methods(http.MethodDelete) + apiRouter.HandleFunc("/skins/{username}", ctx.DeleteSkinByUsername).Methods(http.MethodDelete) // 404 router.NotFoundHandler = http.HandlerFunc(NotFound) @@ -226,7 +223,7 @@ func (ctx *Skinsystem) Textures(response http.ResponseWriter, request *http.Requ texturesProp := mojangTextures.DecodeTextures() if texturesProp == nil { - ctx.Emit("skinsystem.error", errors.New("unable to find textures property")) + ctx.Emit("skinsystem:error", errors.New("unable to find textures property")) apiServerError(response) return } @@ -330,28 +327,6 @@ func (ctx *Skinsystem) DeleteSkinByUsername(resp http.ResponseWriter, req *http. ctx.deleteSkin(skin, err, resp) } -func (ctx *Skinsystem) AuthenticationMiddleware(handler http.Handler) http.Handler { - return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { - // TODO: decide on how I would cover this with logging - // ctx.Logger.IncCounter("authentication.challenge", 1) - err := ctx.Auth.Check(req) - if err != nil { - if _, ok := err.(*auth.Unauthorized); ok { - // ctx.Logger.IncCounter("authentication.failed", 1) - apiForbidden(resp, err.Error()) - } else { - // ctx.Logger.Error("Unknown error on validating api request: :err", wd.ErrParam(err)) - apiServerError(resp) - } - - return - } - - // ctx.Logger.IncCounter("authentication.success", 1) - handler.ServeHTTP(resp, req) - }) -} - func (ctx *Skinsystem) deleteSkin(skin *model.Skin, err error, resp http.ResponseWriter) { if err != nil { if _, ok := err.(*SkinNotFoundError); ok { diff --git a/http/skinsystem_test.go b/http/skinsystem_test.go index 78bc29c..89f317d 100644 --- a/http/skinsystem_test.go +++ b/http/skinsystem_test.go @@ -20,7 +20,6 @@ import ( "github.com/stretchr/testify/suite" "github.com/elyby/chrly/api/mojang" - "github.com/elyby/chrly/auth" "github.com/elyby/chrly/model" ) @@ -95,15 +94,6 @@ func (m *mojangTexturesProviderMock) GetForUsername(username string) (*mojang.Si return result, args.Error(1) } -type authCheckerMock struct { - mock.Mock -} - -func (m *authCheckerMock) Check(req *http.Request) error { - args := m.Called(req) - return args.Error(0) -} - type skinsystemTestSuite struct { suite.Suite @@ -131,7 +121,7 @@ func (suite *skinsystemTestSuite) SetupTest() { SkinsRepo: suite.SkinsRepository, CapesRepo: suite.CapesRepository, MojangTexturesProvider: suite.MojangTexturesProvider, - Auth: suite.Auth, + Authenticator: suite.Auth, Emitter: suite.Emitter, } } @@ -215,6 +205,8 @@ var skinsTestsCases = []*skinsystemTestCase{ func (suite *skinsystemTestSuite) TestSkin() { for _, testCase := range skinsTestsCases { suite.RunSubTest(testCase.Name, func() { + suite.Emitter.On("Emit", "skinsystem:before_request", mock.Anything) + suite.Emitter.On("Emit", "skinsystem:after_request", mock.Anything, mock.Anything) testCase.BeforeTest(suite) req := httptest.NewRequest("GET", "http://chrly/skins/mock_username", nil) @@ -227,6 +219,8 @@ func (suite *skinsystemTestSuite) TestSkin() { } suite.RunSubTest("Pass username with png extension", func() { + suite.Emitter.On("Emit", "skinsystem:before_request", mock.Anything) + suite.Emitter.On("Emit", "skinsystem:after_request", mock.Anything, mock.Anything) suite.SkinsRepository.On("FindByUsername", "mock_username").Return(createSkinModel("mock_username", false), nil) req := httptest.NewRequest("GET", "http://chrly/skins/mock_username.png", nil) @@ -243,6 +237,8 @@ func (suite *skinsystemTestSuite) TestSkin() { func (suite *skinsystemTestSuite) TestSkinGET() { for _, testCase := range skinsTestsCases { suite.RunSubTest(testCase.Name, func() { + suite.Emitter.On("Emit", "skinsystem:before_request", mock.Anything) + suite.Emitter.On("Emit", "skinsystem:after_request", mock.Anything, mock.Anything) testCase.BeforeTest(suite) req := httptest.NewRequest("GET", "http://chrly/skins?name=mock_username", nil) @@ -255,6 +251,9 @@ func (suite *skinsystemTestSuite) TestSkinGET() { } suite.RunSubTest("Do not pass name param", func() { + suite.Emitter.On("Emit", "skinsystem:before_request", mock.Anything) + suite.Emitter.On("Emit", "skinsystem:after_request", mock.Anything, mock.Anything) + req := httptest.NewRequest("GET", "http://chrly/skins", nil) w := httptest.NewRecorder() @@ -318,6 +317,8 @@ var capesTestsCases = []*skinsystemTestCase{ func (suite *skinsystemTestSuite) TestCape() { for _, testCase := range capesTestsCases { suite.RunSubTest(testCase.Name, func() { + suite.Emitter.On("Emit", "skinsystem:before_request", mock.Anything) + suite.Emitter.On("Emit", "skinsystem:after_request", mock.Anything, mock.Anything) testCase.BeforeTest(suite) req := httptest.NewRequest("GET", "http://chrly/cloaks/mock_username", nil) @@ -330,6 +331,8 @@ func (suite *skinsystemTestSuite) TestCape() { } suite.RunSubTest("Pass username with png extension", func() { + suite.Emitter.On("Emit", "skinsystem:before_request", mock.Anything) + suite.Emitter.On("Emit", "skinsystem:after_request", mock.Anything, mock.Anything) suite.CapesRepository.On("FindByUsername", "mock_username").Return(createCapeModel(), nil) req := httptest.NewRequest("GET", "http://chrly/cloaks/mock_username.png", nil) @@ -348,6 +351,8 @@ func (suite *skinsystemTestSuite) TestCape() { func (suite *skinsystemTestSuite) TestCapeGET() { for _, testCase := range capesTestsCases { suite.RunSubTest(testCase.Name, func() { + suite.Emitter.On("Emit", "skinsystem:before_request", mock.Anything) + suite.Emitter.On("Emit", "skinsystem:after_request", mock.Anything, mock.Anything) testCase.BeforeTest(suite) req := httptest.NewRequest("GET", "http://chrly/cloaks?name=mock_username", nil) @@ -360,6 +365,9 @@ func (suite *skinsystemTestSuite) TestCapeGET() { } suite.RunSubTest("Do not pass name param", func() { + suite.Emitter.On("Emit", "skinsystem:before_request", mock.Anything) + suite.Emitter.On("Emit", "skinsystem:after_request", mock.Anything, mock.Anything) + req := httptest.NewRequest("GET", "http://chrly/cloaks", nil) w := httptest.NewRecorder() @@ -486,6 +494,8 @@ var texturesTestsCases = []*skinsystemTestCase{ func (suite *skinsystemTestSuite) TestTextures() { for _, testCase := range texturesTestsCases { suite.RunSubTest(testCase.Name, func() { + suite.Emitter.On("Emit", "skinsystem:before_request", mock.Anything) + suite.Emitter.On("Emit", "skinsystem:after_request", mock.Anything, mock.Anything) testCase.BeforeTest(suite) req := httptest.NewRequest("GET", "http://chrly/textures/mock_username", nil) @@ -609,6 +619,8 @@ var signedTexturesTestsCases = []*signedTexturesTestCase{ func (suite *skinsystemTestSuite) TestSignedTextures() { for _, testCase := range signedTexturesTestsCases { suite.RunSubTest(testCase.Name, func() { + suite.Emitter.On("Emit", "skinsystem:before_request", mock.Anything) + suite.Emitter.On("Emit", "skinsystem:after_request", mock.Anything, mock.Anything) testCase.BeforeTest(suite) var target string @@ -817,7 +829,9 @@ var postSkinTestsCases = []*postSkinTestCase{ func (suite *skinsystemTestSuite) TestPostSkin() { for _, testCase := range postSkinTestsCases { suite.RunSubTest(testCase.Name, func() { - suite.Auth.On("Check", mock.Anything).Return(nil) + suite.Emitter.On("Emit", "skinsystem:before_request", mock.Anything) + suite.Emitter.On("Emit", "skinsystem:after_request", mock.Anything, mock.Anything) + suite.Auth.On("Authenticate", mock.Anything).Return(nil) testCase.BeforeTest(suite) req := httptest.NewRequest("POST", "http://chrly/api/skins", testCase.Form) @@ -831,7 +845,9 @@ func (suite *skinsystemTestSuite) TestPostSkin() { } suite.RunSubTest("Get errors about required fields", func() { - suite.Auth.On("Check", mock.Anything).Return(nil) + suite.Emitter.On("Emit", "skinsystem:before_request", mock.Anything) + suite.Emitter.On("Emit", "skinsystem:after_request", mock.Anything, mock.Anything) + suite.Auth.On("Authenticate", mock.Anything).Return(nil) req := httptest.NewRequest("POST", "http://chrly/api/skins", bytes.NewBufferString(url.Values{ "mojangTextures": {"someBase64EncodedString"}, @@ -878,11 +894,13 @@ func (suite *skinsystemTestSuite) TestPostSkin() { }) suite.RunSubTest("Send request without authorization", func() { + suite.Emitter.On("Emit", "skinsystem:before_request", mock.Anything) + suite.Emitter.On("Emit", "skinsystem:after_request", mock.Anything, mock.Anything) req := httptest.NewRequest("POST", "http://chrly/api/skins", nil) req.Header.Add("Authorization", "Bearer invalid.jwt.token") w := httptest.NewRecorder() - suite.Auth.On("Check", mock.Anything).Return(&auth.Unauthorized{Reason: "Cannot parse passed JWT token"}) + suite.Auth.On("Authenticate", mock.Anything).Return(errors.New("Cannot parse passed JWT token")) suite.App.CreateHandler().ServeHTTP(w, req) @@ -896,7 +914,9 @@ func (suite *skinsystemTestSuite) TestPostSkin() { }) suite.RunSubTest("Upload textures with skin as file", func() { - suite.Auth.On("Check", mock.Anything).Return(nil) + suite.Emitter.On("Emit", "skinsystem:before_request", mock.Anything) + suite.Emitter.On("Emit", "skinsystem:after_request", mock.Anything, mock.Anything) + suite.Auth.On("Authenticate", mock.Anything).Return(nil) inputBody := &bytes.Buffer{} writer := multipart.NewWriter(inputBody) @@ -940,7 +960,9 @@ func (suite *skinsystemTestSuite) TestPostSkin() { func (suite *skinsystemTestSuite) TestDeleteByUserId() { suite.RunSubTest("Delete skin by its identity id", func() { - suite.Auth.On("Check", mock.Anything).Return(nil) + suite.Emitter.On("Emit", "skinsystem:before_request", mock.Anything) + suite.Emitter.On("Emit", "skinsystem:after_request", mock.Anything, mock.Anything) + suite.Auth.On("Authenticate", mock.Anything).Return(nil) suite.SkinsRepository.On("FindByUserId", 1).Return(createSkinModel("mock_username", false), nil) suite.SkinsRepository.On("RemoveByUserId", 1).Once().Return(nil) @@ -957,7 +979,9 @@ func (suite *skinsystemTestSuite) TestDeleteByUserId() { }) suite.RunSubTest("Try to remove not exists identity id", func() { - suite.Auth.On("Check", mock.Anything).Return(nil) + suite.Emitter.On("Emit", "skinsystem:before_request", mock.Anything) + suite.Emitter.On("Emit", "skinsystem:after_request", mock.Anything, mock.Anything) + suite.Auth.On("Authenticate", mock.Anything).Return(nil) suite.SkinsRepository.On("FindByUserId", 1).Return(nil, &SkinNotFoundError{Who: "unknown"}) req := httptest.NewRequest("DELETE", "http://chrly/api/skins/id:1", nil) @@ -981,7 +1005,9 @@ func (suite *skinsystemTestSuite) TestDeleteByUserId() { func (suite *skinsystemTestSuite) TestDeleteByUsername() { suite.RunSubTest("Delete skin by its identity username", func() { - suite.Auth.On("Check", mock.Anything).Return(nil) + suite.Emitter.On("Emit", "skinsystem:before_request", mock.Anything) + suite.Emitter.On("Emit", "skinsystem:after_request", mock.Anything, mock.Anything) + suite.Auth.On("Authenticate", mock.Anything).Return(nil) suite.SkinsRepository.On("FindByUsername", "mock_username").Return(createSkinModel("mock_username", false), nil) suite.SkinsRepository.On("RemoveByUserId", 1).Once().Return(nil) @@ -998,7 +1024,9 @@ func (suite *skinsystemTestSuite) TestDeleteByUsername() { }) suite.RunSubTest("Try to remove not exists identity username", func() { - suite.Auth.On("Check", mock.Anything).Return(nil) + suite.Emitter.On("Emit", "skinsystem:before_request", mock.Anything) + suite.Emitter.On("Emit", "skinsystem:after_request", mock.Anything, mock.Anything) + suite.Auth.On("Authenticate", mock.Anything).Return(nil) suite.SkinsRepository.On("FindByUsername", "mock_username").Return(nil, &SkinNotFoundError{Who: "mock_username"}) req := httptest.NewRequest("DELETE", "http://chrly/api/skins/mock_username", nil)