From 7e07d1b3c2558d639c0464fb87bbd56282ead5b9 Mon Sep 17 00:00:00 2001 From: "Bogdan I. Bursuc" Date: Thu, 16 Mar 2017 18:02:36 +0200 Subject: [PATCH 1/4] Remove Auth module --- scripts/generate_mocks.sh | 23 ---- server/apns/mocks_router_gen_test.go | 12 -- server/auth/accessmanager.go | 21 ---- server/auth/accessmanager_test.go | 63 ---------- server/auth/allow_all_accessmanager.go | 18 --- server/auth/logger.go | 9 -- server/auth/mocks_auth_gen_test.go | 41 ------- server/auth/rest_accessmanager.go | 57 --------- server/connector/mocks_router_gen_test.go | 12 -- server/fcm/mocks_router_gen_test.go | 12 -- server/gubled.go | 10 +- server/gubled_test.go | 2 - server/mocks_auth_gen_test.go | 41 ------- server/mocks_router_gen_test.go | 12 -- server/rest/mocks_router_gen_test.go | 12 -- server/router/errors.go | 20 --- server/router/mocks_auth_gen_test.go | 41 ------- server/router/mocks_router_gen_test.go | 12 -- server/router/route_test.go | 5 +- server/router/router.go | 55 ++------- server/router/router_test.go | 123 ++----------------- server/service/mocks_router_gen_test.go | 12 -- server/sms/mocks_router_gen_test.go | 12 -- server/websocket/mocks_auth_gen_test.go | 41 ------- server/websocket/mocks_router_gen_test.go | 12 -- server/websocket/websocket_connector.go | 33 +---- server/websocket/websocket_connector_test.go | 45 +------ 27 files changed, 35 insertions(+), 721 deletions(-) delete mode 100644 server/auth/accessmanager.go delete mode 100644 server/auth/accessmanager_test.go delete mode 100644 server/auth/allow_all_accessmanager.go delete mode 100644 server/auth/logger.go delete mode 100644 server/auth/mocks_auth_gen_test.go delete mode 100644 server/auth/rest_accessmanager.go delete mode 100644 server/mocks_auth_gen_test.go delete mode 100644 server/router/mocks_auth_gen_test.go delete mode 100644 server/websocket/mocks_auth_gen_test.go diff --git a/scripts/generate_mocks.sh b/scripts/generate_mocks.sh index 9336ab32..a141711e 100755 --- a/scripts/generate_mocks.sh +++ b/scripts/generate_mocks.sh @@ -48,11 +48,6 @@ $MOCKGEN -self_package router -package router \ github.com/cosminrentea/gobbler/server/kvstore \ KVStore & -$MOCKGEN -self_package router -package router \ - -destination server/router/mocks_auth_gen_test.go \ - github.com/cosminrentea/gobbler/server/auth \ - AccessManager & - $MOCKGEN -self_package router -package router \ -destination server/router/mocks_checker_gen_test.go \ github.com/docker/distribution/health \ @@ -113,11 +108,6 @@ $MOCKGEN -package server \ github.com/cosminrentea/gobbler/server/router \ Router & -$MOCKGEN -self_package server -package server \ - -destination server/mocks_auth_gen_test.go \ - github.com/cosminrentea/gobbler/server/auth \ - AccessManager & - $MOCKGEN -self_package server -package server \ -destination server/mocks_store_gen_test.go \ github.com/cosminrentea/gobbler/server/store \ @@ -128,14 +118,6 @@ $MOCKGEN -package server \ github.com/cosminrentea/gobbler/server/apns \ Pusher & -# server/auth mocks -$MOCKGEN -self_package auth -package auth \ - -destination server/auth/mocks_auth_gen_test.go \ - github.com/cosminrentea/gobbler/server/auth \ - AccessManager -replace "server/auth/mocks_auth_gen_test.go" \ - "auth \"github.com\/cosminrentea\/gobbler\/server\/auth\"" \ - "auth\." # server/connector mocks $MOCKGEN -self_package connector -package connector \ @@ -175,11 +157,6 @@ $MOCKGEN -self_package websocket -package websocket \ github.com/cosminrentea/gobbler/server/store \ MessageStore & -$MOCKGEN -self_package websocket -package websocket \ - -destination server/websocket/mocks_auth_gen_test.go \ - github.com/cosminrentea/gobbler/server/auth \ - AccessManager & - # server/rest Mocks $MOCKGEN -package rest \ -destination server/rest/mocks_router_gen_test.go \ diff --git a/server/apns/mocks_router_gen_test.go b/server/apns/mocks_router_gen_test.go index 33a666dd..f935254f 100644 --- a/server/apns/mocks_router_gen_test.go +++ b/server/apns/mocks_router_gen_test.go @@ -5,7 +5,6 @@ package apns import ( protocol "github.com/cosminrentea/gobbler/protocol" - auth "github.com/cosminrentea/gobbler/server/auth" cluster "github.com/cosminrentea/gobbler/server/cluster" kvstore "github.com/cosminrentea/gobbler/server/kvstore" router "github.com/cosminrentea/gobbler/server/router" @@ -34,17 +33,6 @@ func (_m *MockRouter) EXPECT() *_MockRouterRecorder { return _m.recorder } -func (_m *MockRouter) AccessManager() (auth.AccessManager, error) { - ret := _m.ctrl.Call(_m, "AccessManager") - ret0, _ := ret[0].(auth.AccessManager) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -func (_mr *_MockRouterRecorder) AccessManager() *gomock.Call { - return _mr.mock.ctrl.RecordCall(_mr.mock, "AccessManager") -} - func (_m *MockRouter) Cluster() *cluster.Cluster { ret := _m.ctrl.Call(_m, "Cluster") ret0, _ := ret[0].(*cluster.Cluster) diff --git a/server/auth/accessmanager.go b/server/auth/accessmanager.go deleted file mode 100644 index 0d2c700b..00000000 --- a/server/auth/accessmanager.go +++ /dev/null @@ -1,21 +0,0 @@ -package auth - -import ( - "github.com/cosminrentea/gobbler/protocol" -) - -// AccessType permission required by the user -type AccessType int - -const ( - // READ permission - READ AccessType = iota - - // WRITE permission - WRITE -) - -// AccessManager interface allows to provide a custom authentication mechanism -type AccessManager interface { - IsAllowed(accessType AccessType, userID string, path protocol.Path) bool -} diff --git a/server/auth/accessmanager_test.go b/server/auth/accessmanager_test.go deleted file mode 100644 index 949f1db5..00000000 --- a/server/auth/accessmanager_test.go +++ /dev/null @@ -1,63 +0,0 @@ -package auth - -import ( - "github.com/stretchr/testify/assert" - "net/http" - "net/http/httptest" - "testing" -) - -func Test_AllowAllAccessManager(t *testing.T) { - a := assert.New(t) - am := AccessManager(NewAllowAllAccessManager(true)) - a.True(am.IsAllowed(READ, "userid", "/path")) - - am = AccessManager(NewAllowAllAccessManager(false)) - a.False(am.IsAllowed(READ, "userid", "/path")) -} - -func Test_RestAccessManagerAllowed(t *testing.T) { - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Write([]byte("true")) - })) - - defer ts.Close() - a := assert.New(t) - am := NewRestAccessManager(ts.URL) - a.True(am.IsAllowed(READ, "foo", "/foo")) - a.True(am.IsAllowed(WRITE, "foo", "/foo")) -} - -func Test_RestAccessManagerNotAllowed(t *testing.T) { - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Write([]byte("false")) - })) - - defer ts.Close() - am := NewRestAccessManager(ts.URL) - a := assert.New(t) - a.False(am.IsAllowed(READ, "user", "/foo")) -} - -func Test_RestAccessManagerNotAllowedWithServerNotStarted(t *testing.T) { - ts := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Write([]byte("false")) - })) - - defer ts.Close() - am := NewRestAccessManager(ts.URL) - a := assert.New(t) - a.False(am.IsAllowed(READ, "user", "/foo")) -} - -func Test_RestAccessManagerNotAllowedHttpReturningStatusForbidden(t *testing.T) { - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusForbidden) - })) - - defer ts.Close() - a := assert.New(t) - am := NewRestAccessManager(ts.URL) - a.False(am.IsAllowed(READ, "foo", "/foo")) - a.False(am.IsAllowed(WRITE, "foo", "/foo")) -} diff --git a/server/auth/allow_all_accessmanager.go b/server/auth/allow_all_accessmanager.go deleted file mode 100644 index aa057efb..00000000 --- a/server/auth/allow_all_accessmanager.go +++ /dev/null @@ -1,18 +0,0 @@ -package auth - -import ( - "github.com/cosminrentea/gobbler/protocol" -) - -//AllowAllAccessManager is a dummy implementation that grants access for everything. -type AllowAllAccessManager bool - -//NewAllowAllAccessManager returns a new AllowAllAccessManager (depending on the passed parameter, always true or always false) -func NewAllowAllAccessManager(allowAll bool) AllowAllAccessManager { - return AllowAllAccessManager(allowAll) -} - -//IsAllowed returns always the same value, given at construction time (true or false). -func (am AllowAllAccessManager) IsAllowed(accessType AccessType, userID string, path protocol.Path) bool { - return bool(am) -} diff --git a/server/auth/logger.go b/server/auth/logger.go deleted file mode 100644 index fe1b963b..00000000 --- a/server/auth/logger.go +++ /dev/null @@ -1,9 +0,0 @@ -package auth - -import ( - log "github.com/Sirupsen/logrus" -) - -var logger = log.WithFields(log.Fields{ - "module": "accessManager", -}) diff --git a/server/auth/mocks_auth_gen_test.go b/server/auth/mocks_auth_gen_test.go deleted file mode 100644 index 2507e052..00000000 --- a/server/auth/mocks_auth_gen_test.go +++ /dev/null @@ -1,41 +0,0 @@ -// Automatically generated by MockGen. DO NOT EDIT! -// Source: github.com/cosminrentea/gobbler/server/auth (interfaces: AccessManager) - -package auth - -import ( - protocol "github.com/cosminrentea/gobbler/protocol" - - gomock "github.com/golang/mock/gomock" -) - -// Mock of AccessManager interface -type MockAccessManager struct { - ctrl *gomock.Controller - recorder *_MockAccessManagerRecorder -} - -// Recorder for MockAccessManager (not exported) -type _MockAccessManagerRecorder struct { - mock *MockAccessManager -} - -func NewMockAccessManager(ctrl *gomock.Controller) *MockAccessManager { - mock := &MockAccessManager{ctrl: ctrl} - mock.recorder = &_MockAccessManagerRecorder{mock} - return mock -} - -func (_m *MockAccessManager) EXPECT() *_MockAccessManagerRecorder { - return _m.recorder -} - -func (_m *MockAccessManager) IsAllowed(_param0 AccessType, _param1 string, _param2 protocol.Path) bool { - ret := _m.ctrl.Call(_m, "IsAllowed", _param0, _param1, _param2) - ret0, _ := ret[0].(bool) - return ret0 -} - -func (_mr *_MockAccessManagerRecorder) IsAllowed(arg0, arg1, arg2 interface{}) *gomock.Call { - return _mr.mock.ctrl.RecordCall(_mr.mock, "IsAllowed", arg0, arg1, arg2) -} diff --git a/server/auth/rest_accessmanager.go b/server/auth/rest_accessmanager.go deleted file mode 100644 index 7defb99b..00000000 --- a/server/auth/rest_accessmanager.go +++ /dev/null @@ -1,57 +0,0 @@ -package auth - -import ( - "github.com/cosminrentea/gobbler/protocol" - - log "github.com/Sirupsen/logrus" - - "io/ioutil" - "net/http" - "net/url" -) - -// RestAccessManager is a url for which the access is allowed or not. -type RestAccessManager string - -// NewRestAccessManager returns a new RestAccessManager. -func NewRestAccessManager(url string) RestAccessManager { - return RestAccessManager(url) -} - -// IsAllowed is an implementation of the AccessManager interface. -// The boolean result is based on matching between the desired AccessType, the userId and the path. -func (ram RestAccessManager) IsAllowed(accessType AccessType, userId string, path protocol.Path) bool { - - u, _ := url.Parse(string(ram)) - q := u.Query() - if accessType == READ { - q.Set("type", "read") - } else { - q.Set("type", "write") - } - - q.Set("userId", userId) - q.Set("path", string(path)) - - resp, err := http.DefaultClient.Get(u.String()) - - if err != nil { - logger.WithError(err).WithField("module", "RestAccessManager").Warn("Write message failed") - return false - } - defer resp.Body.Close() - responseBody, err := ioutil.ReadAll(resp.Body) - - if err != nil || resp.StatusCode != 200 { - logger.WithError(err).WithField("httpCode", resp.StatusCode).Info("Error getting permission") - logger.WithField("responseBody", responseBody).Debug("HTTP Response Body") - return false - } - logger.WithFields(log.Fields{ - "access_type": accessType, - "userId": userId, - "path": path, - "responseBody": string(responseBody), - }).Debug("Access allowed") - return "true" == string(responseBody) -} diff --git a/server/connector/mocks_router_gen_test.go b/server/connector/mocks_router_gen_test.go index f0d00dd0..36e82db5 100644 --- a/server/connector/mocks_router_gen_test.go +++ b/server/connector/mocks_router_gen_test.go @@ -5,7 +5,6 @@ package connector import ( protocol "github.com/cosminrentea/gobbler/protocol" - auth "github.com/cosminrentea/gobbler/server/auth" cluster "github.com/cosminrentea/gobbler/server/cluster" kvstore "github.com/cosminrentea/gobbler/server/kvstore" router "github.com/cosminrentea/gobbler/server/router" @@ -34,17 +33,6 @@ func (_m *MockRouter) EXPECT() *_MockRouterRecorder { return _m.recorder } -func (_m *MockRouter) AccessManager() (auth.AccessManager, error) { - ret := _m.ctrl.Call(_m, "AccessManager") - ret0, _ := ret[0].(auth.AccessManager) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -func (_mr *_MockRouterRecorder) AccessManager() *gomock.Call { - return _mr.mock.ctrl.RecordCall(_mr.mock, "AccessManager") -} - func (_m *MockRouter) Cluster() *cluster.Cluster { ret := _m.ctrl.Call(_m, "Cluster") ret0, _ := ret[0].(*cluster.Cluster) diff --git a/server/fcm/mocks_router_gen_test.go b/server/fcm/mocks_router_gen_test.go index f9ad3755..078e3ea5 100644 --- a/server/fcm/mocks_router_gen_test.go +++ b/server/fcm/mocks_router_gen_test.go @@ -5,7 +5,6 @@ package fcm import ( protocol "github.com/cosminrentea/gobbler/protocol" - auth "github.com/cosminrentea/gobbler/server/auth" cluster "github.com/cosminrentea/gobbler/server/cluster" kvstore "github.com/cosminrentea/gobbler/server/kvstore" router "github.com/cosminrentea/gobbler/server/router" @@ -34,17 +33,6 @@ func (_m *MockRouter) EXPECT() *_MockRouterRecorder { return _m.recorder } -func (_m *MockRouter) AccessManager() (auth.AccessManager, error) { - ret := _m.ctrl.Call(_m, "AccessManager") - ret0, _ := ret[0].(auth.AccessManager) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -func (_mr *_MockRouterRecorder) AccessManager() *gomock.Call { - return _mr.mock.ctrl.RecordCall(_mr.mock, "AccessManager") -} - func (_m *MockRouter) Cluster() *cluster.Cluster { ret := _m.ctrl.Call(_m, "Cluster") ret0, _ := ret[0].(*cluster.Cluster) diff --git a/server/gubled.go b/server/gubled.go index 4a021aae..f3f3204b 100644 --- a/server/gubled.go +++ b/server/gubled.go @@ -6,7 +6,6 @@ import ( "github.com/cosminrentea/gobbler/logformatter" "github.com/cosminrentea/gobbler/protocol" "github.com/cosminrentea/gobbler/server/apns" - "github.com/cosminrentea/gobbler/server/auth" "github.com/cosminrentea/gobbler/server/cluster" "github.com/cosminrentea/gobbler/server/fcm" "github.com/cosminrentea/gobbler/server/kvstore" @@ -59,12 +58,6 @@ var ValidateStoragePath = func() error { return nil } -// CreateAccessManager is a func which returns a auth.AccessManager implementation -// (currently: AllowAllAccessManager). -var CreateAccessManager = func() auth.AccessManager { - return auth.NewAllowAllAccessManager(true) -} - // CreateKVStore is a func which returns a kvstore.KVStore implementation // (currently, based on guble configuration). var CreateKVStore = func() kvstore.KVStore { @@ -254,7 +247,6 @@ func Main() { func StartService() *service.Service { //TODO StartService could return an error in case it fails to start - accessManager := CreateAccessManager() messageStore := CreateMessageStore() kvStore := CreateKVStore() @@ -276,7 +268,7 @@ func StartService() *service.Service { logger.Info("Starting in standalone-mode") } - r := router.New(accessManager, messageStore, kvStore, cl) + r := router.New(messageStore, kvStore, cl) websrv := webserver.New(*Config.HttpListen) srv := service.New(r, websrv). diff --git a/server/gubled_test.go b/server/gubled_test.go index e29c04a9..8b85a933 100644 --- a/server/gubled_test.go +++ b/server/gubled_test.go @@ -136,10 +136,8 @@ func TestStartServiceModules(t *testing.T) { func initRouterMock() *MockRouter { routerMock := NewMockRouter(testutil.MockCtrl) routerMock.EXPECT().Cluster().Return(nil).AnyTimes() - amMock := NewMockAccessManager(testutil.MockCtrl) msMock := NewMockMessageStore(testutil.MockCtrl) - routerMock.EXPECT().AccessManager().Return(amMock, nil).AnyTimes() routerMock.EXPECT().MessageStore().Return(msMock, nil).AnyTimes() return routerMock diff --git a/server/mocks_auth_gen_test.go b/server/mocks_auth_gen_test.go deleted file mode 100644 index 3f045c6d..00000000 --- a/server/mocks_auth_gen_test.go +++ /dev/null @@ -1,41 +0,0 @@ -// Automatically generated by MockGen. DO NOT EDIT! -// Source: github.com/cosminrentea/gobbler/server/auth (interfaces: AccessManager) - -package server - -import ( - protocol "github.com/cosminrentea/gobbler/protocol" - auth "github.com/cosminrentea/gobbler/server/auth" - gomock "github.com/golang/mock/gomock" -) - -// Mock of AccessManager interface -type MockAccessManager struct { - ctrl *gomock.Controller - recorder *_MockAccessManagerRecorder -} - -// Recorder for MockAccessManager (not exported) -type _MockAccessManagerRecorder struct { - mock *MockAccessManager -} - -func NewMockAccessManager(ctrl *gomock.Controller) *MockAccessManager { - mock := &MockAccessManager{ctrl: ctrl} - mock.recorder = &_MockAccessManagerRecorder{mock} - return mock -} - -func (_m *MockAccessManager) EXPECT() *_MockAccessManagerRecorder { - return _m.recorder -} - -func (_m *MockAccessManager) IsAllowed(_param0 auth.AccessType, _param1 string, _param2 protocol.Path) bool { - ret := _m.ctrl.Call(_m, "IsAllowed", _param0, _param1, _param2) - ret0, _ := ret[0].(bool) - return ret0 -} - -func (_mr *_MockAccessManagerRecorder) IsAllowed(arg0, arg1, arg2 interface{}) *gomock.Call { - return _mr.mock.ctrl.RecordCall(_mr.mock, "IsAllowed", arg0, arg1, arg2) -} diff --git a/server/mocks_router_gen_test.go b/server/mocks_router_gen_test.go index f3bc02e8..1453c854 100644 --- a/server/mocks_router_gen_test.go +++ b/server/mocks_router_gen_test.go @@ -5,7 +5,6 @@ package server import ( protocol "github.com/cosminrentea/gobbler/protocol" - auth "github.com/cosminrentea/gobbler/server/auth" cluster "github.com/cosminrentea/gobbler/server/cluster" kvstore "github.com/cosminrentea/gobbler/server/kvstore" router "github.com/cosminrentea/gobbler/server/router" @@ -34,17 +33,6 @@ func (_m *MockRouter) EXPECT() *_MockRouterRecorder { return _m.recorder } -func (_m *MockRouter) AccessManager() (auth.AccessManager, error) { - ret := _m.ctrl.Call(_m, "AccessManager") - ret0, _ := ret[0].(auth.AccessManager) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -func (_mr *_MockRouterRecorder) AccessManager() *gomock.Call { - return _mr.mock.ctrl.RecordCall(_mr.mock, "AccessManager") -} - func (_m *MockRouter) Cluster() *cluster.Cluster { ret := _m.ctrl.Call(_m, "Cluster") ret0, _ := ret[0].(*cluster.Cluster) diff --git a/server/rest/mocks_router_gen_test.go b/server/rest/mocks_router_gen_test.go index 3d8c8e42..ff913206 100644 --- a/server/rest/mocks_router_gen_test.go +++ b/server/rest/mocks_router_gen_test.go @@ -5,7 +5,6 @@ package rest import ( protocol "github.com/cosminrentea/gobbler/protocol" - auth "github.com/cosminrentea/gobbler/server/auth" cluster "github.com/cosminrentea/gobbler/server/cluster" kvstore "github.com/cosminrentea/gobbler/server/kvstore" router "github.com/cosminrentea/gobbler/server/router" @@ -34,17 +33,6 @@ func (_m *MockRouter) EXPECT() *_MockRouterRecorder { return _m.recorder } -func (_m *MockRouter) AccessManager() (auth.AccessManager, error) { - ret := _m.ctrl.Call(_m, "AccessManager") - ret0, _ := ret[0].(auth.AccessManager) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -func (_mr *_MockRouterRecorder) AccessManager() *gomock.Call { - return _mr.mock.ctrl.RecordCall(_mr.mock, "AccessManager") -} - func (_m *MockRouter) Cluster() *cluster.Cluster { ret := _m.ctrl.Call(_m, "Cluster") ret0, _ := ret[0].(*cluster.Cluster) diff --git a/server/router/errors.go b/server/router/errors.go index 8c96edcd..3dffd11d 100644 --- a/server/router/errors.go +++ b/server/router/errors.go @@ -1,9 +1,6 @@ package router import ( - "github.com/cosminrentea/gobbler/protocol" - "github.com/cosminrentea/gobbler/server/auth" - "errors" "fmt" ) @@ -24,23 +21,6 @@ var ( ErrQueueFull = errors.New("Route queue is full. Route is closed.") ) -// PermissionDeniedError is returned when AccessManager denies a user request for a topic -type PermissionDeniedError struct { - - // userId of request - UserID string - - // accessType requested(READ/WRITE) - AccessType auth.AccessType - - // requested topic - Path protocol.Path -} - -func (e *PermissionDeniedError) Error() string { - return fmt.Sprintf("Access Denied for user=[%s] on path=[%s] for Operation=[%s]", e.UserID, e.Path, e.AccessType) -} - // ModuleStoppingError is returned when the module is stopping type ModuleStoppingError struct { Name string diff --git a/server/router/mocks_auth_gen_test.go b/server/router/mocks_auth_gen_test.go deleted file mode 100644 index 1d655a80..00000000 --- a/server/router/mocks_auth_gen_test.go +++ /dev/null @@ -1,41 +0,0 @@ -// Automatically generated by MockGen. DO NOT EDIT! -// Source: github.com/cosminrentea/gobbler/server/auth (interfaces: AccessManager) - -package router - -import ( - protocol "github.com/cosminrentea/gobbler/protocol" - auth "github.com/cosminrentea/gobbler/server/auth" - gomock "github.com/golang/mock/gomock" -) - -// Mock of AccessManager interface -type MockAccessManager struct { - ctrl *gomock.Controller - recorder *_MockAccessManagerRecorder -} - -// Recorder for MockAccessManager (not exported) -type _MockAccessManagerRecorder struct { - mock *MockAccessManager -} - -func NewMockAccessManager(ctrl *gomock.Controller) *MockAccessManager { - mock := &MockAccessManager{ctrl: ctrl} - mock.recorder = &_MockAccessManagerRecorder{mock} - return mock -} - -func (_m *MockAccessManager) EXPECT() *_MockAccessManagerRecorder { - return _m.recorder -} - -func (_m *MockAccessManager) IsAllowed(_param0 auth.AccessType, _param1 string, _param2 protocol.Path) bool { - ret := _m.ctrl.Call(_m, "IsAllowed", _param0, _param1, _param2) - ret0, _ := ret[0].(bool) - return ret0 -} - -func (_mr *_MockAccessManagerRecorder) IsAllowed(arg0, arg1, arg2 interface{}) *gomock.Call { - return _mr.mock.ctrl.RecordCall(_mr.mock, "IsAllowed", arg0, arg1, arg2) -} diff --git a/server/router/mocks_router_gen_test.go b/server/router/mocks_router_gen_test.go index fe115f8d..3582109b 100644 --- a/server/router/mocks_router_gen_test.go +++ b/server/router/mocks_router_gen_test.go @@ -5,7 +5,6 @@ package router import ( protocol "github.com/cosminrentea/gobbler/protocol" - auth "github.com/cosminrentea/gobbler/server/auth" cluster "github.com/cosminrentea/gobbler/server/cluster" kvstore "github.com/cosminrentea/gobbler/server/kvstore" @@ -34,17 +33,6 @@ func (_m *MockRouter) EXPECT() *_MockRouterRecorder { return _m.recorder } -func (_m *MockRouter) AccessManager() (auth.AccessManager, error) { - ret := _m.ctrl.Call(_m, "AccessManager") - ret0, _ := ret[0].(auth.AccessManager) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -func (_mr *_MockRouterRecorder) AccessManager() *gomock.Call { - return _mr.mock.ctrl.RecordCall(_mr.mock, "AccessManager") -} - func (_m *MockRouter) Cluster() *cluster.Cluster { ret := _m.ctrl.Call(_m, "Cluster") ret0, _ := ret[0].(*cluster.Cluster) diff --git a/server/router/route_test.go b/server/router/route_test.go index 66dd87a9..dc0c89e9 100644 --- a/server/router/route_test.go +++ b/server/router/route_test.go @@ -7,12 +7,11 @@ import ( "testing" "time" - "github.com/golang/mock/gomock" "github.com/cosminrentea/gobbler/protocol" - "github.com/cosminrentea/gobbler/server/auth" "github.com/cosminrentea/gobbler/server/kvstore" "github.com/cosminrentea/gobbler/server/store" "github.com/cosminrentea/gobbler/testutil" + "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" ) @@ -407,7 +406,7 @@ func TestRoute_Provide_MultipleFetch(t *testing.T) { memoryKV := kvstore.NewMemoryKVStore() msMock := NewMockMessageStore(ctrl) - router := New(auth.AllowAllAccessManager(true), msMock, memoryKV, nil) + router := New(msMock, memoryKV, nil) if startable, ok := router.(startable); ok { startable.Start() diff --git a/server/router/router.go b/server/router/router.go index 1dd2b0fb..c5a515d5 100644 --- a/server/router/router.go +++ b/server/router/router.go @@ -14,7 +14,6 @@ import ( "net/http" "github.com/cosminrentea/gobbler/protocol" - "github.com/cosminrentea/gobbler/server/auth" "github.com/cosminrentea/gobbler/server/cluster" "github.com/cosminrentea/gobbler/server/kvstore" "github.com/cosminrentea/gobbler/server/store" @@ -36,7 +35,6 @@ type Router interface { Fetch(*store.FetchRequest) error GetSubscribers(topic string) ([]byte, error) - AccessManager() (auth.AccessManager, error) MessageStore() (store.MessageStore, error) KVStore() (kvstore.KVStore, error) Cluster() *cluster.Cluster @@ -59,16 +57,15 @@ type router struct { stopping bool // Flag: the router is in stopping process and no incoming messages are accepted wg sync.WaitGroup // Add any operation that we need to wait upon here - accessManager auth.AccessManager - messageStore store.MessageStore - kvStore kvstore.KVStore - cluster *cluster.Cluster + messageStore store.MessageStore + kvStore kvstore.KVStore + cluster *cluster.Cluster sync.RWMutex } // New returns a pointer to Router -func New(accessManager auth.AccessManager, messageStore store.MessageStore, kvStore kvstore.KVStore, cluster *cluster.Cluster) Router { +func New(messageStore store.MessageStore, kvStore kvstore.KVStore, cluster *cluster.Cluster) Router { return &router{ routes: make(map[protocol.Path][]*Route), @@ -77,10 +74,9 @@ func New(accessManager auth.AccessManager, messageStore store.MessageStore, kvSt unsubscribeC: make(chan subRequest, unsubscribeChannelCapacity), stopC: make(chan bool, 1), - accessManager: accessManager, - messageStore: messageStore, - kvStore: kvStore, - cluster: cluster, + messageStore: messageStore, + kvStore: kvStore, + cluster: cluster, } } @@ -133,7 +129,7 @@ func (router *router) Stop() error { } func (router *router) Check() error { - if router.accessManager == nil || router.messageStore == nil || router.kvStore == nil { + if router.messageStore == nil || router.kvStore == nil { logger.WithError(ErrServiceNotProvided).Error("Some mandatory services are not provided") return ErrServiceNotProvided } @@ -167,10 +163,6 @@ func (router *router) HandleMessage(message *protocol.Message) error { return err } - if !router.accessManager.IsAllowed(auth.WRITE, message.UserID, message.Path) { - return &PermissionDeniedError{UserID: message.UserID, AccessType: auth.WRITE, Path: message.Path} - } - var nodeID uint8 if router.cluster != nil { nodeID = router.cluster.Config.ID @@ -197,22 +189,12 @@ func (router *router) HandleMessage(message *protocol.Message) error { } func (router *router) Subscribe(r *Route) (*Route, error) { - logger.WithFields(log.Fields{ - "accessManager": router.accessManager, - "route": r, - }).Debug("Subscribe") + logger.WithFields(log.Fields{"route": r}).Debug("Subscribe") if err := router.isStopping(); err != nil { return nil, err } - userID := r.Get("user_id") - routePath := r.Path - - accessAllowed := router.accessManager.IsAllowed(auth.READ, userID, routePath) - if !accessAllowed { - return r, &PermissionDeniedError{UserID: userID, AccessType: auth.READ, Path: routePath} - } req := subRequest{ route: r, doneC: make(chan bool), @@ -225,10 +207,7 @@ func (router *router) Subscribe(r *Route) (*Route, error) { // Subscribe adds a route to the subscribers. If there is already a route with same Application Id and Path, it will be replaced. func (router *router) Unsubscribe(r *Route) { - logger.WithFields(log.Fields{ - "accessManager": router.accessManager, - "route": r, - }).Debug("Unsubscribe") + logger.WithFields(log.Fields{"route": r}).Debug("Unsubscribe") req := subRequest{ route: r, @@ -303,9 +282,9 @@ func (router *router) unsubscribe(r *Route) { } func (router *router) panicIfInternalDependenciesAreNil() { - if router.accessManager == nil || router.kvStore == nil || router.messageStore == nil { - panic(fmt.Sprintf("router: the internal dependencies marked with `true` are not set: AccessManager=%v, KVStore=%v, MessageStore=%v", - router.accessManager == nil, router.kvStore == nil, router.messageStore == nil)) + if router.kvStore == nil || router.messageStore == nil { + panic(fmt.Sprintf("router: the internal dependencies marked with `true` are not set: KVStore=%v, MessageStore=%v", + router.kvStore == nil, router.messageStore == nil)) } } @@ -419,14 +398,6 @@ func (router *router) Fetch(req *store.FetchRequest) error { return nil } -// AccessManager returns the `accessManager` provided for the router -func (router *router) AccessManager() (auth.AccessManager, error) { - if router.accessManager == nil { - return nil, ErrServiceNotProvided - } - return router.accessManager, nil -} - // MessageStore returns the `messageStore` provided for the router func (router *router) MessageStore() (store.MessageStore, error) { if router.messageStore == nil { diff --git a/server/router/router_test.go b/server/router/router_test.go index b73b99cd..ec8501e9 100644 --- a/server/router/router_test.go +++ b/server/router/router_test.go @@ -6,7 +6,6 @@ import ( "time" "github.com/cosminrentea/gobbler/protocol" - "github.com/cosminrentea/gobbler/server/auth" "github.com/cosminrentea/gobbler/server/kvstore" "github.com/cosminrentea/gobbler/server/store" "github.com/cosminrentea/gobbler/server/store/dummystore" @@ -46,7 +45,7 @@ func TestRouter_AddAndRemoveRoutes(t *testing.T) { a := assert.New(t) // Given a Router - router, _, _, _ := aStartedRouter() + router, _, _ := aStartedRouter() // when i add two routes in the same path routeBlah1, _ := router.Subscribe(NewRoute( @@ -94,102 +93,11 @@ func TestRouter_AddAndRemoveRoutes(t *testing.T) { a.Nil(router.routes[protocol.Path("/foo")]) } -func TestRouter_SubscribeNotAllowed(t *testing.T) { - ctrl, finish := testutil.NewMockCtrl(t) - defer finish() - a := assert.New(t) - - am := NewMockAccessManager(ctrl) - msMock := NewMockMessageStore(ctrl) - kvsMock := NewMockKVStore(ctrl) - - am.EXPECT().IsAllowed(auth.READ, "user01", protocol.Path("/blah")).Return(false) - - router := New(am, msMock, kvsMock, nil).(*router) - router.Start() - - _, e := router.Subscribe(NewRoute( - RouteConfig{ - RouteParams: RouteParams{"application_id": "appid01", "user_id": "user01"}, - Path: protocol.Path("/blah"), - ChannelSize: chanSize, - }, - )) - - // default TestAccessManager denies all - a.NotNil(e) - - // now add permissions - am.EXPECT().IsAllowed(auth.READ, "user01", protocol.Path("/blah")).Return(true) - - // and user shall be allowed to subscribe - _, e = router.Subscribe(NewRoute( - RouteConfig{ - RouteParams: RouteParams{"application_id": "appid01", "user_id": "user01"}, - Path: protocol.Path("/blah"), - ChannelSize: chanSize, - }, - )) - - a.Nil(e) -} - -func TestRouter_HandleMessageNotAllowed(t *testing.T) { - ctrl, finish := testutil.NewMockCtrl(t) - defer finish() - a := assert.New(t) - - amMock := NewMockAccessManager(ctrl) - msMock := NewMockMessageStore(ctrl) - kvsMock := NewMockKVStore(ctrl) - - // Given a Router with route - router, r := aRouterRoute(chanSize) - router.accessManager = amMock - router.messageStore = msMock - router.kvStore = kvsMock - - amMock.EXPECT().IsAllowed(auth.WRITE, r.Get("user_id"), r.Path).Return(false) - - // when i send a message to the route - err := router.HandleMessage(&protocol.Message{ - Path: r.Path, - Body: aTestByteMessage, - UserID: r.Get("user_id"), - }) - - // an error shall be returned - a.Error(err) - - // and when permission is granted - id, ts := uint64(2), time.Now().Unix() - - amMock.EXPECT().IsAllowed(auth.WRITE, r.Get("user_id"), r.Path).Return(true) - msMock.EXPECT(). - StoreMessage(gomock.Any(), gomock.Any()). - Do(func(m *protocol.Message, nodeID uint8) (int, error) { - m.ID = id - m.Time = ts - m.NodeID = nodeID - return len(m.Bytes()), nil - }) - - // sending message - err = router.HandleMessage(&protocol.Message{ - Path: r.Path, - Body: aTestByteMessage, - UserID: r.Get("user_id"), - }) - - // shall give no error - a.NoError(err) -} - func TestRouter_ReplacingOfRoutesMatchingAppID(t *testing.T) { a := assert.New(t) // Given a Router with a route - router, _, _, _ := aStartedRouter() + router, _, _ := aStartedRouter() matcherFunc := func(route, other RouteConfig, keys ...string) bool { return route.Path == other.Path && route.Get("application_id") == other.Get("application_id") @@ -250,7 +158,7 @@ func TestRouter_RoutingWithSubTopics(t *testing.T) { a := assert.New(t) // Given a Router with route - router, _, _, _ := aStartedRouter() + router, _, _ := aStartedRouter() msMock := NewMockMessageStore(ctrl) router.messageStore = msMock @@ -388,7 +296,7 @@ func TestRouter_CleanShutdown(t *testing.T) { }). AnyTimes() - router, _, _, _ := aStartedRouter() + router, _, _ := aStartedRouter() router.messageStore = msMock route, err := router.Subscribe(NewRoute( @@ -454,38 +362,28 @@ func TestRouter_Check(t *testing.T) { defer finish() a := assert.New(t) - amMock := NewMockAccessManager(ctrl) msMock := NewMockMessageStore(ctrl) kvsMock := NewMockKVStore(ctrl) msCheckerMock := newMSChecker() kvsCheckerMock := newKVSChecker() // Given a Multiplexer with route - router, _, _, _ := aStartedRouter() + router, _, _ := aStartedRouter() // Test 0: Router is healthy by default a.Nil(router.Check()) - // Test 1a: Given accessManager is nil, then router's Check returns error - router.accessManager = nil - router.messageStore = msMock - router.kvStore = kvsMock - a.NotNil(router.Check()) - // Test 1b: Given messageStore is nil, then router's Check returns error - router.accessManager = amMock router.messageStore = nil router.kvStore = kvsMock a.NotNil(router.Check()) // Test 1c: Given kvStore is nil, then router's Check return error - router.accessManager = amMock router.messageStore = msMock router.kvStore = nil a.NotNil(router.Check()) // Test 2: Given mocked store dependencies, both healthy - router.accessManager = amMock router.messageStore = msCheckerMock router.kvStore = kvsCheckerMock @@ -510,21 +408,20 @@ func TestRouter_Check(t *testing.T) { func TestPanicOnInternalDependencies(t *testing.T) { defer testutil.ExpectPanic(t) - router := New(nil, nil, nil, nil).(*router) + router := New(nil, nil, nil).(*router) router.panicIfInternalDependenciesAreNil() } -func aStartedRouter() (*router, auth.AccessManager, store.MessageStore, kvstore.KVStore) { - am := auth.NewAllowAllAccessManager(true) +func aStartedRouter() (*router, store.MessageStore, kvstore.KVStore) { kvs := kvstore.NewMemoryKVStore() ms := dummystore.New(kvs) - router := New(am, ms, kvs, nil).(*router) + router := New(ms, kvs, nil).(*router) router.Start() - return router, am, ms, kvs + return router, ms, kvs } func aRouterRoute(unused int) (*router, *Route) { - router, _, _, _ := aStartedRouter() + router, _, _ := aStartedRouter() route, _ := router.Subscribe(NewRoute( RouteConfig{ RouteParams: RouteParams{"application_id": "appid01", "user_id": "user01"}, diff --git a/server/service/mocks_router_gen_test.go b/server/service/mocks_router_gen_test.go index b823b621..eda9f818 100644 --- a/server/service/mocks_router_gen_test.go +++ b/server/service/mocks_router_gen_test.go @@ -5,7 +5,6 @@ package service import ( protocol "github.com/cosminrentea/gobbler/protocol" - auth "github.com/cosminrentea/gobbler/server/auth" cluster "github.com/cosminrentea/gobbler/server/cluster" kvstore "github.com/cosminrentea/gobbler/server/kvstore" router "github.com/cosminrentea/gobbler/server/router" @@ -34,17 +33,6 @@ func (_m *MockRouter) EXPECT() *_MockRouterRecorder { return _m.recorder } -func (_m *MockRouter) AccessManager() (auth.AccessManager, error) { - ret := _m.ctrl.Call(_m, "AccessManager") - ret0, _ := ret[0].(auth.AccessManager) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -func (_mr *_MockRouterRecorder) AccessManager() *gomock.Call { - return _mr.mock.ctrl.RecordCall(_mr.mock, "AccessManager") -} - func (_m *MockRouter) Cluster() *cluster.Cluster { ret := _m.ctrl.Call(_m, "Cluster") ret0, _ := ret[0].(*cluster.Cluster) diff --git a/server/sms/mocks_router_gen_test.go b/server/sms/mocks_router_gen_test.go index 41eb1c1b..2bf08add 100644 --- a/server/sms/mocks_router_gen_test.go +++ b/server/sms/mocks_router_gen_test.go @@ -5,7 +5,6 @@ package sms import ( protocol "github.com/cosminrentea/gobbler/protocol" - auth "github.com/cosminrentea/gobbler/server/auth" cluster "github.com/cosminrentea/gobbler/server/cluster" kvstore "github.com/cosminrentea/gobbler/server/kvstore" router "github.com/cosminrentea/gobbler/server/router" @@ -34,17 +33,6 @@ func (_m *MockRouter) EXPECT() *_MockRouterRecorder { return _m.recorder } -func (_m *MockRouter) AccessManager() (auth.AccessManager, error) { - ret := _m.ctrl.Call(_m, "AccessManager") - ret0, _ := ret[0].(auth.AccessManager) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -func (_mr *_MockRouterRecorder) AccessManager() *gomock.Call { - return _mr.mock.ctrl.RecordCall(_mr.mock, "AccessManager") -} - func (_m *MockRouter) Cluster() *cluster.Cluster { ret := _m.ctrl.Call(_m, "Cluster") ret0, _ := ret[0].(*cluster.Cluster) diff --git a/server/websocket/mocks_auth_gen_test.go b/server/websocket/mocks_auth_gen_test.go deleted file mode 100644 index 289ab3aa..00000000 --- a/server/websocket/mocks_auth_gen_test.go +++ /dev/null @@ -1,41 +0,0 @@ -// Automatically generated by MockGen. DO NOT EDIT! -// Source: github.com/cosminrentea/gobbler/server/auth (interfaces: AccessManager) - -package websocket - -import ( - protocol "github.com/cosminrentea/gobbler/protocol" - auth "github.com/cosminrentea/gobbler/server/auth" - gomock "github.com/golang/mock/gomock" -) - -// Mock of AccessManager interface -type MockAccessManager struct { - ctrl *gomock.Controller - recorder *_MockAccessManagerRecorder -} - -// Recorder for MockAccessManager (not exported) -type _MockAccessManagerRecorder struct { - mock *MockAccessManager -} - -func NewMockAccessManager(ctrl *gomock.Controller) *MockAccessManager { - mock := &MockAccessManager{ctrl: ctrl} - mock.recorder = &_MockAccessManagerRecorder{mock} - return mock -} - -func (_m *MockAccessManager) EXPECT() *_MockAccessManagerRecorder { - return _m.recorder -} - -func (_m *MockAccessManager) IsAllowed(_param0 auth.AccessType, _param1 string, _param2 protocol.Path) bool { - ret := _m.ctrl.Call(_m, "IsAllowed", _param0, _param1, _param2) - ret0, _ := ret[0].(bool) - return ret0 -} - -func (_mr *_MockAccessManagerRecorder) IsAllowed(arg0, arg1, arg2 interface{}) *gomock.Call { - return _mr.mock.ctrl.RecordCall(_mr.mock, "IsAllowed", arg0, arg1, arg2) -} diff --git a/server/websocket/mocks_router_gen_test.go b/server/websocket/mocks_router_gen_test.go index e132bebe..d7d65383 100644 --- a/server/websocket/mocks_router_gen_test.go +++ b/server/websocket/mocks_router_gen_test.go @@ -5,7 +5,6 @@ package websocket import ( protocol "github.com/cosminrentea/gobbler/protocol" - auth "github.com/cosminrentea/gobbler/server/auth" cluster "github.com/cosminrentea/gobbler/server/cluster" kvstore "github.com/cosminrentea/gobbler/server/kvstore" router "github.com/cosminrentea/gobbler/server/router" @@ -34,17 +33,6 @@ func (_m *MockRouter) EXPECT() *_MockRouterRecorder { return _m.recorder } -func (_m *MockRouter) AccessManager() (auth.AccessManager, error) { - ret := _m.ctrl.Call(_m, "AccessManager") - ret0, _ := ret[0].(auth.AccessManager) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -func (_mr *_MockRouterRecorder) AccessManager() *gomock.Call { - return _mr.mock.ctrl.RecordCall(_mr.mock, "AccessManager") -} - func (_m *MockRouter) Cluster() *cluster.Cluster { ret := _m.ctrl.Call(_m, "Cluster") ret0, _ := ret[0].(*cluster.Cluster) diff --git a/server/websocket/websocket_connector.go b/server/websocket/websocket_connector.go index b4d14978..f11c4217 100644 --- a/server/websocket/websocket_connector.go +++ b/server/websocket/websocket_connector.go @@ -2,7 +2,6 @@ package websocket import ( "github.com/cosminrentea/gobbler/protocol" - "github.com/cosminrentea/gobbler/server/auth" "github.com/cosminrentea/gobbler/server/router" log "github.com/Sirupsen/logrus" @@ -21,21 +20,15 @@ var webSocketUpgrader = websocket.Upgrader{ // WSHandler is a struct used for handling websocket connections on a certain prefix. type WSHandler struct { - router router.Router - prefix string - accessManager auth.AccessManager + router router.Router + prefix string } // NewWSHandler returns a new WSHandler. func NewWSHandler(router router.Router, prefix string) (*WSHandler, error) { - accessManager, err := router.AccessManager() - if err != nil { - return nil, err - } return &WSHandler{ - router: router, - prefix: prefix, - accessManager: accessManager, + router: router, + prefix: prefix, }, nil } @@ -121,9 +114,6 @@ func (ws *WebSocket) Start() error { func (ws *WebSocket) sendLoop() { for raw := range ws.sendChannel { - if !ws.checkAccess(raw) { - continue - } if err := ws.Send(raw); err != nil { logger.WithFields(log.Fields{ "userId": ws.userID, @@ -137,21 +127,6 @@ func (ws *WebSocket) sendLoop() { } } -func (ws *WebSocket) checkAccess(raw []byte) bool { - if len(raw) > 0 && raw[0] == byte('/') { - path := getPathFromRawMessage(raw) - - logger.WithFields(log.Fields{ - "userID": ws.userID, - "path": path, - }).Debug("Received msg") - - return len(path) == 0 || ws.accessManager.IsAllowed(auth.READ, ws.userID, path) - - } - return true -} - func getPathFromRawMessage(raw []byte) protocol.Path { i := strings.Index(string(raw), ",") return protocol.Path(raw[:i]) diff --git a/server/websocket/websocket_connector_test.go b/server/websocket/websocket_connector_test.go index 59c97c7f..a039f9fe 100644 --- a/server/websocket/websocket_connector_test.go +++ b/server/websocket/websocket_connector_test.go @@ -2,10 +2,10 @@ package websocket import ( "github.com/cosminrentea/gobbler/protocol" - "github.com/cosminrentea/gobbler/server/auth" "github.com/cosminrentea/gobbler/server/router" "github.com/cosminrentea/gobbler/server/store" "github.com/cosminrentea/gobbler/testutil" + "github.com/smancke/guble/server/auth" "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" @@ -88,39 +88,6 @@ func Test_AnIncomingMessageIsDelivered(t *testing.T) { time.Sleep(time.Millisecond * 2) } -func Test_AnIncomingMessageIsNotAllowed(t *testing.T) { - ctrl, finish := testutil.NewMockCtrl(t) - defer finish() - - wsconn, routerMock, _ := createDefaultMocks([]string{}) - - tam := NewMockAccessManager(ctrl) - tam.EXPECT().IsAllowed(auth.READ, "testuser", protocol.Path("/foo")).Return(false) - handler := NewWebSocket( - testWSHandler(routerMock, tam), - wsconn, - "testuser", - ) - go func() { - handler.Start() - }() - time.Sleep(time.Millisecond * 2) - - handler.sendChannel <- aTestMessage.Bytes() - time.Sleep(time.Millisecond * 2) - //nothing shall have been sent - - //now allow - tam.EXPECT().IsAllowed(auth.READ, "testuser", protocol.Path("/foo")).Return(true) - - wsconn.EXPECT().Send(aTestMessage.Bytes()) - - time.Sleep(time.Millisecond * 2) - - handler.sendChannel <- aTestMessage.Bytes() - time.Sleep(time.Millisecond * 2) -} - func Test_BadCommands(t *testing.T) { _, finish := testutil.NewMockCtrl(t) defer finish() @@ -160,13 +127,11 @@ func TestExtractUserId(t *testing.T) { } func testWSHandler( - routerMock *MockRouter, - accessManager auth.AccessManager) *WSHandler { + routerMock *MockRouter) *WSHandler { return &WSHandler{ - router: routerMock, - prefix: "/prefix", - accessManager: accessManager, + router: routerMock, + prefix: "/prefix", } } @@ -180,7 +145,7 @@ func runNewWebSocket( accessManager = auth.NewAllowAllAccessManager(true) } websocket := NewWebSocket( - testWSHandler(routerMock, accessManager), + testWSHandler(routerMock), wsconn, "testuser", ) From 22ef638ebd937b0c01f10d001cc93aa7c2025760 Mon Sep 17 00:00:00 2001 From: "Bogdan I. Bursuc" Date: Mon, 20 Mar 2017 11:03:23 +0200 Subject: [PATCH 2/4] Remove auth usage from tests --- server/sms/utils_test.go | 13 ++++++------- server/websocket/websocket_connector_test.go | 15 +++++---------- 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/server/sms/utils_test.go b/server/sms/utils_test.go index 38e17699..218bc54c 100644 --- a/server/sms/utils_test.go +++ b/server/sms/utils_test.go @@ -3,16 +3,16 @@ package sms import ( "encoding/json" "fmt" + "io/ioutil" + "math/rand" + "testing" + "time" + "github.com/cosminrentea/gobbler/protocol" - "github.com/cosminrentea/gobbler/server/auth" "github.com/cosminrentea/gobbler/server/kvstore" "github.com/cosminrentea/gobbler/server/router" "github.com/cosminrentea/gobbler/server/store/dummystore" "github.com/stretchr/testify/assert" - "io/ioutil" - "math/rand" - "testing" - "time" ) var ( @@ -121,9 +121,8 @@ func createGateway(t *testing.T, kvStore kvstore.KVStore) *gateway { sender := createNexmoSender(t) config := createConfig() msgStore := dummystore.New(kvStore) - accessManager := auth.NewAllowAllAccessManager(true) - unstartedRouter := router.New(accessManager, msgStore, kvStore, nil) + unstartedRouter := router.New(msgStore, kvStore, nil) gw, err := New(unstartedRouter, sender, config) a.NoError(err) diff --git a/server/websocket/websocket_connector_test.go b/server/websocket/websocket_connector_test.go index a039f9fe..32286564 100644 --- a/server/websocket/websocket_connector_test.go +++ b/server/websocket/websocket_connector_test.go @@ -5,7 +5,6 @@ import ( "github.com/cosminrentea/gobbler/server/router" "github.com/cosminrentea/gobbler/server/store" "github.com/cosminrentea/gobbler/testutil" - "github.com/smancke/guble/server/auth" "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" @@ -54,7 +53,7 @@ func Test_WebSocket_SubscribeAndUnsubscribe(t *testing.T) { Send([]byte("#" + protocol.SUCCESS_CANCELED + " /foo")). Do(doneGroup) - websocket := runNewWebSocket(wsconn, routerMock, messageStore, nil) + websocket := runNewWebSocket(wsconn, routerMock, messageStore) wg.Wait() a.Equal(1, len(websocket.receivers)) @@ -71,7 +70,7 @@ func Test_SendMessage(t *testing.T) { routerMock.EXPECT().HandleMessage(messageMatcher{path: "/path", message: "Hello, this is a test", header: `{"key": "value"}`}) wsconn.EXPECT().Send([]byte("#send")) - runNewWebSocket(wsconn, routerMock, messageStore, nil) + runNewWebSocket(wsconn, routerMock, messageStore) } func Test_AnIncomingMessageIsDelivered(t *testing.T) { @@ -82,7 +81,7 @@ func Test_AnIncomingMessageIsDelivered(t *testing.T) { wsconn.EXPECT().Send(aTestMessage.Bytes()) - handler := runNewWebSocket(wsconn, routerMock, messageStore, nil) + handler := runNewWebSocket(wsconn, routerMock, messageStore) handler.sendChannel <- aTestMessage.Bytes() time.Sleep(time.Millisecond * 2) @@ -114,7 +113,7 @@ func Test_BadCommands(t *testing.T) { return nil }).AnyTimes() - runNewWebSocket(wsconn, routerMock, messageStore, nil) + runNewWebSocket(wsconn, routerMock, messageStore) wg.Wait() assert.Equal(t, len(badRequests), counter, "expected number of bad requests does not match") @@ -138,12 +137,8 @@ func testWSHandler( func runNewWebSocket( wsconn *MockWSConnection, routerMock *MockRouter, - messageStore store.MessageStore, - accessManager auth.AccessManager) *WebSocket { + messageStore store.MessageStore) *WebSocket { - if accessManager == nil { - accessManager = auth.NewAllowAllAccessManager(true) - } websocket := NewWebSocket( testWSHandler(routerMock), wsconn, From 764d442ca3f0ec6d9a7634351248b1b942409875 Mon Sep 17 00:00:00 2001 From: "Bogdan I. Bursuc" Date: Mon, 20 Mar 2017 15:33:56 +0200 Subject: [PATCH 3/4] PR changes --- server/router/router.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/router/router.go b/server/router/router.go index c5a515d5..2fb0879d 100644 --- a/server/router/router.go +++ b/server/router/router.go @@ -189,7 +189,7 @@ func (router *router) HandleMessage(message *protocol.Message) error { } func (router *router) Subscribe(r *Route) (*Route, error) { - logger.WithFields(log.Fields{"route": r}).Debug("Subscribe") + logger.WithField("route", r).Debug("Subscribe") if err := router.isStopping(); err != nil { return nil, err @@ -207,7 +207,7 @@ func (router *router) Subscribe(r *Route) (*Route, error) { // Subscribe adds a route to the subscribers. If there is already a route with same Application Id and Path, it will be replaced. func (router *router) Unsubscribe(r *Route) { - logger.WithFields(log.Fields{"route": r}).Debug("Unsubscribe") + logger.WithFields("route", r).Debug("Unsubscribe") req := subRequest{ route: r, From 19e45dc0b3f719ff29c4354b2872ac647b30f319 Mon Sep 17 00:00:00 2001 From: "Bogdan I. Bursuc" Date: Mon, 20 Mar 2017 15:52:20 +0200 Subject: [PATCH 4/4] Fix error --- server/router/router.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/router/router.go b/server/router/router.go index 2fb0879d..2de270e0 100644 --- a/server/router/router.go +++ b/server/router/router.go @@ -207,7 +207,7 @@ func (router *router) Subscribe(r *Route) (*Route, error) { // Subscribe adds a route to the subscribers. If there is already a route with same Application Id and Path, it will be replaced. func (router *router) Unsubscribe(r *Route) { - logger.WithFields("route", r).Debug("Unsubscribe") + logger.WithField("route", r).Debug("Unsubscribe") req := subRequest{ route: r,