From: Jeff Halter Date: Tue, 7 Jun 2022 02:41:07 +0000 (-0700) Subject: Refactor and cleanup to improve testability X-Git-Url: https://git.r.bdr.sh/rbdr/mobius/commitdiff_plain/d4c152a4dba0eec7c8ecd13732900909f51b1c97 Refactor and cleanup to improve testability --- diff --git a/hotline/client_conn.go b/hotline/client_conn.go index 65e18ee..c1ae4e3 100644 --- a/hotline/client_conn.go +++ b/hotline/client_conn.go @@ -3,8 +3,8 @@ package hotline import ( "encoding/binary" "golang.org/x/crypto/bcrypt" + "io" "math/big" - "net" ) type byClientID []*ClientConn @@ -21,9 +21,37 @@ func (s byClientID) Less(i, j int) bool { return s[i].uint16ID() < s[j].uint16ID() } +const template = `Nickname: %s +Name: %s +Account: %s +Address: %s + +-------- File Downloads --------- + +%s + +------- Folder Downloads -------- + +None. + +--------- File Uploads ---------- + +None. + +-------- Folder Uploads --------- + +None. + +------- Waiting Downloads ------- + +None. + + ` + // ClientConn represents a client connected to a Server type ClientConn struct { - Connection net.Conn + Connection io.ReadWriteCloser + RemoteAddr string ID *[]byte Icon *[]byte Flags *[]byte @@ -67,15 +95,6 @@ func (cc *ClientConn) handleTransaction(transaction *Transaction) error { return nil } } - if !authorize(cc.Account.Access, handler.Access) { - cc.Server.Logger.Infow( - "Unauthorized Action", - "Account", cc.Account.Login, "UserName", string(cc.UserName), "RequestType", handler.Name, - ) - cc.Server.outbox <- cc.NewErrReply(transaction, handler.DenyMsg) - - return nil - } cc.Server.Logger.Infow( "Received Transaction", @@ -160,7 +179,7 @@ func (cc *ClientConn) Disconnect() { cc.notifyOthers(*NewTransaction(tranNotifyDeleteUser, nil, NewField(fieldUserID, *cc.ID))) if err := cc.Connection.Close(); err != nil { - cc.Server.Logger.Errorw("error closing client connection", "RemoteAddr", cc.Connection.RemoteAddr()) + cc.Server.Logger.Errorw("error closing client connection", "RemoteAddr", cc.RemoteAddr) } } diff --git a/hotline/flattened_file_object_test.go b/hotline/flattened_file_object_test.go index 656af68..4b3fdf6 100644 --- a/hotline/flattened_file_object_test.go +++ b/hotline/flattened_file_object_test.go @@ -2,7 +2,6 @@ package hotline import ( "fmt" - "github.com/davecgh/go-spew/spew" "github.com/stretchr/testify/assert" "os" "testing" @@ -99,8 +98,6 @@ func TestFlatFileInformationFork_UnmarshalBinary(t *testing.T) { t.Run(tt.name, func(t *testing.T) { ffif := &FlatFileInformationFork{} tt.wantErr(t, ffif.UnmarshalBinary(tt.args.b), fmt.Sprintf("UnmarshalBinary(%v)", tt.args.b)) - - spew.Dump(ffif) }) } } diff --git a/hotline/handshake.go b/hotline/handshake.go index 66bf643..39d9cab 100644 --- a/hotline/handshake.go +++ b/hotline/handshake.go @@ -4,7 +4,7 @@ import ( "bytes" "encoding/binary" "errors" - "net" + "io" ) type handshake struct { @@ -33,9 +33,14 @@ type handshake struct { // Description Size Data Note // Protocol ID 4 TRTP // Error code 4 Error code returned by the server (0 = no error) -func Handshake(conn net.Conn, buf []byte) error { +func Handshake(conn io.ReadWriter) error { + handshakeBuf := make([]byte, 12) + if _, err := io.ReadFull(conn, handshakeBuf); err != nil { + return err + } + var h handshake - r := bytes.NewReader(buf) + r := bytes.NewReader(handshakeBuf) if err := binary.Read(r, binary.BigEndian, &h); err != nil { return err } diff --git a/hotline/server.go b/hotline/server.go index f164df1..fb06001 100644 --- a/hotline/server.go +++ b/hotline/server.go @@ -131,7 +131,7 @@ func (s *Server) sendTransaction(t Transaction) error { "IsReply", t.IsReply, "type", handler.Name, "sentBytes", n, - "remoteAddr", client.Connection.RemoteAddr(), + "remoteAddr", client.RemoteAddr, ) return nil } @@ -156,7 +156,7 @@ func (s *Server) Serve(ctx context.Context, cancelRoot context.CancelFunc, ln ne } }() go func() { - if err := s.handleNewConnection(conn); err != nil { + if err := s.handleNewConnection(conn, conn.RemoteAddr().String()); err != nil { if err == io.EOF { s.Logger.Infow("Client disconnected", "RemoteAddr", conn.RemoteAddr()) } else { @@ -319,7 +319,7 @@ func (s *Server) writeThreadedNews() error { return err } -func (s *Server) NewClientConn(conn net.Conn) *ClientConn { +func (s *Server) NewClientConn(conn net.Conn, remoteAddr string) *ClientConn { s.mux.Lock() defer s.mux.Unlock() @@ -334,6 +334,7 @@ func (s *Server) NewClientConn(conn net.Conn) *ClientConn { AutoReply: []byte{}, Transfers: make(map[int][]*FileTransfer), Agreed: false, + RemoteAddr: remoteAddr, } *s.NextGuestID++ ID := *s.NextGuestID @@ -493,14 +494,10 @@ func dontPanic(logger *zap.SugaredLogger) { } // handleNewConnection takes a new net.Conn and performs the initial login sequence -func (s *Server) handleNewConnection(conn net.Conn) error { +func (s *Server) handleNewConnection(conn net.Conn, remoteAddr string) error { defer dontPanic(s.Logger) - handshakeBuf := make([]byte, 12) - if _, err := io.ReadFull(conn, handshakeBuf); err != nil { - return err - } - if err := Handshake(conn, handshakeBuf); err != nil { + if err := Handshake(conn); err != nil { return err } @@ -519,7 +516,7 @@ func (s *Server) handleNewConnection(conn net.Conn) error { return err } - c := s.NewClientConn(conn) + c := s.NewClientConn(conn, remoteAddr) defer c.Disconnect() encodedLogin := clientLogin.GetField(fieldUserLogin).Data @@ -561,7 +558,7 @@ func (s *Server) handleNewConnection(conn net.Conn) error { *c.Flags = []byte{0, 2} } - s.Logger.Infow("Client connection received", "login", login, "version", *c.Version, "RemoteAddr", conn.RemoteAddr().String()) + s.Logger.Infow("Client connection received", "login", login, "version", *c.Version, "RemoteAddr", remoteAddr) s.outbox <- c.NewReply(clientLogin, NewField(fieldVersion, []byte{0x00, 0xbe}), diff --git a/hotline/server_blackbox_test.go b/hotline/server_blackbox_test.go index 4e7ea8e..17ad7f4 100644 --- a/hotline/server_blackbox_test.go +++ b/hotline/server_blackbox_test.go @@ -1,51 +1,13 @@ package hotline import ( - "bytes" - "context" - "fmt" - "github.com/davecgh/go-spew/spew" "github.com/stretchr/testify/assert" "go.uber.org/zap" "go.uber.org/zap/zapcore" - "net" "os" "testing" ) -type testCase struct { - name string // test case description - account Account // Account struct for a user that will test transaction will execute under - request *Transaction // transaction that will be sent by the client to the server - setup func() // Optional test-specific setup required for the scenario - teardown func() // Optional test-specific teardown for the scenario - mockHandler map[int]*mockClientHandler -} - -func (tt *testCase) Setup(srv *Server) error { - if err := srv.NewUser(tt.account.Login, tt.account.Name, string(negateString([]byte(tt.account.Password))), *tt.account.Access); err != nil { - return err - } - - if tt.setup != nil { - tt.setup() - } - - return nil -} - -func (tt *testCase) Teardown(srv *Server) error { - if err := srv.DeleteUser(tt.account.Login); err != nil { - return err - } - - if tt.teardown != nil { - tt.teardown() - } - - return nil -} - func NewTestLogger() *zap.SugaredLogger { encoderCfg := zap.NewProductionEncoderConfig() encoderCfg.TimeKey = "timestamp" @@ -63,263 +25,6 @@ func NewTestLogger() *zap.SugaredLogger { return l.Sugar() } -func StartTestServer() (*Server, context.Context, context.CancelFunc) { - ctx, cancelRoot := context.WithCancel(context.Background()) - - FS = &OSFileStore{} - - srv, err := NewServer("test/config/", "localhost", 0, NewTestLogger()) - if err != nil { - panic(err) - } - - go func() { - err := srv.ListenAndServe(ctx, cancelRoot) - if err != nil { - panic(err) - } - }() - - return srv, ctx, cancelRoot -} - -func TestHandshake(t *testing.T) { - mfs := &MockFileStore{} - fh, _ := os.Open("./test/config/Agreement.txt") - mfs.On("Open", "/test/config/Agreement.txt").Return(fh, nil) - fh, _ = os.Open("./test/config/config.yaml") - mfs.On("Open", "/test/config/config.yaml").Return(fh, nil) - FS = mfs - spew.Dump(mfs) - - srv, _, cancelFunc := StartTestServer() - defer cancelFunc() - - port := srv.APIListener.Addr().(*net.TCPAddr).Port - - conn, err := net.Dial("tcp", fmt.Sprintf(":%v", port)) - if err != nil { - t.Fatal(err) - } - defer conn.Close() - - conn.Write([]byte{0x54, 0x52, 0x54, 0x50, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00, 0x02}) - - replyBuf := make([]byte, 8) - _, _ = conn.Read(replyBuf) - - want := []byte{84, 82, 84, 80, 0, 0, 0, 0} - if bytes.Compare(replyBuf, want) != 0 { - t.Errorf("%q, want %q", replyBuf, want) - } - -} - -// func TestLogin(t *testing.T) { -// -// tests := []struct { -// name string -// client *Client -// }{ -// { -// name: "when login is successful", -// client: NewClient("guest", NewTestLogger()), -// }, -// } -// for _, test := range tests { -// t.Run(test.name, func(t *testing.T) { -// -// }) -// } -// } - -func TestNewUser(t *testing.T) { - srv, _, _ := StartTestServer() - - tests := []testCase{ - // { - // name: "a valid new account", - // mockHandler: func() mockClientHandler { - // mh := mockClientHandler{} - // mh.On("Handle", mock.AnythingOfType("*hotline.Client"), mock.MatchedBy(func(t *Transaction) bool { - // println("@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@") - // spew.Dump(t.Type) - // spew.Dump(bytes.Equal(t.Type, []byte{0x01, 0x5e})) - // //if !bytes.Equal(t.GetField(fieldError).Data, []byte("You are not allowed to create new accounts.")) { - // // return false - // //} - // return bytes.Equal(t.Type, []byte{0x01, 0x5e}, - // ) - // })).Return( - // []Transaction{}, nil, - // ) - // - // clientHandlers[tranNewUser] = mh - // return mh - // }(), - // client: func() *Client { - // c := NewClient("testUser", NewTestLogger()) - // return c - // }(), - // teardown: func() { - // _ = srv.DeleteUser("testUser") - // }, - // account: Account{ - // Login: "test", - // Name: "unnamed", - // Password: "test", - // Access: &[]byte{255, 255, 255, 255, 255, 255, 255, 255}, - // }, - // request: NewTransaction( - // tranNewUser, nil, - // NewField(fieldUserLogin, []byte(NegatedUserString([]byte("testUser")))), - // NewField(fieldUserName, []byte("testUserName")), - // NewField(fieldUserPassword, []byte(NegatedUserString([]byte("testPw")))), - // NewField(fieldUserAccess, []byte{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}), - // ), - // want: &Transaction{ - // Fields: []Field{}, - // }, - // }, - // { - // name: "a newUser request from a user without the required access", - // mockHandler: func() *mockClientHandler { - // mh := mockClientHandler{} - // mh.On("Handle", mock.AnythingOfType("*hotline.Client"), mock.MatchedBy(func(t *Transaction) bool { - // if !bytes.Equal(t.GetField(fieldError).Data, []byte("You are not allowed to create new accounts.")) { - // return false - // } - // return bytes.Equal(t.Type, []byte{0x01, 0x5e}) - // })).Return( - // []Transaction{}, nil, - // ) - // return &mh - // }(), - // teardown: func() { - // _ = srv.DeleteUser("testUser") - // }, - // account: Account{ - // Login: "test", - // Name: "unnamed", - // Password: "test", - // Access: &[]byte{0, 0, 0, 0, 0, 0, 0, 0}, - // }, - // request: NewTransaction( - // tranNewUser, nil, - // NewField(fieldUserLogin, []byte(NegatedUserString([]byte("testUser")))), - // NewField(fieldUserName, []byte("testUserName")), - // NewField(fieldUserPassword, []byte(NegatedUserString([]byte("testPw")))), - // NewField(fieldUserAccess, []byte{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}), - // ), - // }, - // { - // name: "when user does not have required permission", - // mockHandler: func() map[int]*mockClientHandler { - // mockHandlers := make(map[int]*mockClientHandler) - // - // mh := mockClientHandler{} - // mh.On("Handle", mock.AnythingOfType("*hotline.Client"), mock.MatchedBy(func(t *Transaction) bool { - // return t.equal(Transaction{ - // Type: []byte{0x01, 0x5e}, - // IsReply: 1, - // ErrorCode: []byte{0, 0, 0, 1}, - // Fields: []Field{ - // NewField(fieldError, []byte("You are not allowed to create new accounts.")), - // }, - // }) - // })).Return( - // []Transaction{}, nil, - // ) - // mockHandlers[tranNewUser] = &mh - // - // return mockHandlers - // }(), - // - // teardown: func() { - // _ = srv.DeleteUser("testUser") - // }, - // account: Account{ - // Login: "test", - // Name: "unnamed", - // Password: "test", - // Access: &[]byte{0, 0, 0, 0, 0, 0, 0, 0}, - // }, - // request: NewTransaction( - // tranNewUser, nil, - // NewField(fieldUserLogin, []byte(NegatedUserString([]byte("testUser")))), - // NewField(fieldUserName, []byte("testUserName")), - // NewField(fieldUserPassword, []byte(NegatedUserString([]byte("testPw")))), - // NewField(fieldUserAccess, []byte{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}), - // ), - // }, - - // { - // name: "a request to create a user that already exists", - // setup: func() { - // - // }, - // teardown: func() { - // _ = srv.DeleteUser("testUser") - // }, - // account: Account{ - // Login: "test", - // Name: "unnamed", - // Password: "test", - // Access: &[]byte{255, 255, 255, 255, 255, 255, 255, 255}, - // }, - // request: NewTransaction( - // tranNewUser, nil, - // NewField(fieldUserLogin, []byte(NegatedUserString([]byte("guest")))), - // NewField(fieldUserName, []byte("testUserName")), - // NewField(fieldUserPassword, []byte(NegatedUserString([]byte("testPw")))), - // NewField(fieldUserAccess, []byte{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}), - // ), - // want: &Transaction{ - // Fields: []Field{ - // NewField(fieldError, []byte("Cannot create account guest because there is already an account with that login.")), - // }, - // }, - // }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - test.Setup(srv) - - // move to Setup? - c := NewClient(test.account.Name, NewTestLogger()) - err := c.JoinServer(fmt.Sprintf(":%v", srv.APIPort()), test.account.Login, test.account.Password) - if err != nil { - t.Errorf("login failed: %v", err) - } - // end move to Setup?? - - for key, value := range test.mockHandler { - c.Handlers[uint16(key)] = value - } - - // send test case request - _ = c.Send(*test.request) - - // time.Sleep(1 * time.Second) - // === - - transactions, _ := readN(c.Connection, 1) - for _, t := range transactions { - _ = c.HandleTransaction(&t) - } - - // === - - for _, handler := range test.mockHandler { - handler.AssertExpectations(t) - } - - test.Teardown(srv) - }) - } -} - // tranAssertEqual compares equality of transactions slices after stripping out the random ID func tranAssertEqual(t *testing.T, tran1, tran2 []Transaction) bool { var newT1 []Transaction diff --git a/hotline/transaction.go b/hotline/transaction.go index e7bfa58..9eb8794 100644 --- a/hotline/transaction.go +++ b/hotline/transaction.go @@ -6,7 +6,6 @@ import ( "fmt" "github.com/jhalter/mobius/concat" "math/rand" - "net" ) const ( @@ -131,29 +130,6 @@ func ReadTransaction(buf []byte) (*Transaction, int, error) { }, tranLen, nil } -func readN(conn net.Conn, n int) ([]Transaction, error) { - buf := make([]byte, 1400) - i := 0 - for { - readLen, err := conn.Read(buf) - if err != nil { - return nil, err - } - - transactions, _, err := readTransactions(buf[:readLen]) - // spew.Fdump(os.Stderr, transactions) - if err != nil { - return nil, err - } - - i += len(transactions) - - if n == i { - return transactions, nil - } - } -} - func readTransactions(buf []byte) ([]Transaction, int, error) { var transactions []Transaction diff --git a/hotline/transaction_handlers.go b/hotline/transaction_handlers.go index 9e524e8..20e7651 100644 --- a/hotline/transaction_handlers.go +++ b/hotline/transaction_handlers.go @@ -16,8 +16,6 @@ import ( ) type TransactionType struct { - Access int // Specifies access privilege required to perform the transaction - DenyMsg string // The error reply message when user does not have access Handler func(*ClientConn, *Transaction) ([]Transaction, error) // function for handling the transaction type Name string // Name of transaction as it will appear in logging RequiredFields []requiredField @@ -45,14 +43,12 @@ var TransactionHandlers = map[uint16]TransactionType{ Name: "tranNotifyDeleteUser", }, tranAgreed: { - Access: accessAlwaysAllow, Name: "tranAgreed", Handler: HandleTranAgreed, }, tranChatSend: { - Access: accessAlwaysAllow, - Handler: HandleChatSend, Name: "tranChatSend", + Handler: HandleChatSend, RequiredFields: []requiredField{ { ID: fieldData, @@ -61,181 +57,130 @@ var TransactionHandlers = map[uint16]TransactionType{ }, }, tranDelNewsArt: { - Access: accessNewsDeleteArt, - DenyMsg: "You are not allowed to delete news articles.", Name: "tranDelNewsArt", Handler: HandleDelNewsArt, }, tranDelNewsItem: { - Access: accessAlwaysAllow, // Granular access enforced inside the handler - // Has multiple access flags: News Delete Folder (37) or News Delete Category (35) - // TODO: Implement inside the handler Name: "tranDelNewsItem", Handler: HandleDelNewsItem, }, tranDeleteFile: { - Access: accessAlwaysAllow, // Granular access enforced inside the handler Name: "tranDeleteFile", Handler: HandleDeleteFile, }, tranDeleteUser: { - Access: accessAlwaysAllow, Name: "tranDeleteUser", Handler: HandleDeleteUser, }, tranDisconnectUser: { - Access: accessDisconUser, - DenyMsg: "You are not allowed to disconnect users.", Name: "tranDisconnectUser", Handler: HandleDisconnectUser, }, tranDownloadFile: { - Access: accessAlwaysAllow, Name: "tranDownloadFile", Handler: HandleDownloadFile, }, tranDownloadFldr: { - Access: accessDownloadFile, // There is no specific access flag for folder vs file download - DenyMsg: "You are not allowed to download files.", Name: "tranDownloadFldr", Handler: HandleDownloadFolder, }, tranGetClientInfoText: { - Access: accessGetClientInfo, - DenyMsg: "You are not allowed to get client info", Name: "tranGetClientInfoText", Handler: HandleGetClientConnInfoText, }, tranGetFileInfo: { - Access: accessAlwaysAllow, Name: "tranGetFileInfo", Handler: HandleGetFileInfo, }, tranGetFileNameList: { - Access: accessAlwaysAllow, Name: "tranGetFileNameList", Handler: HandleGetFileNameList, }, tranGetMsgs: { - Access: accessAlwaysAllow, Name: "tranGetMsgs", Handler: HandleGetMsgs, }, tranGetNewsArtData: { - Access: accessNewsReadArt, - DenyMsg: "You are not allowed to read news.", Name: "tranGetNewsArtData", Handler: HandleGetNewsArtData, }, tranGetNewsArtNameList: { - Access: accessNewsReadArt, - DenyMsg: "You are not allowed to read news.", Name: "tranGetNewsArtNameList", Handler: HandleGetNewsArtNameList, }, tranGetNewsCatNameList: { - Access: accessNewsReadArt, - DenyMsg: "You are not allowed to read news.", Name: "tranGetNewsCatNameList", Handler: HandleGetNewsCatNameList, }, tranGetUser: { - Access: accessAlwaysAllow, Name: "tranGetUser", Handler: HandleGetUser, }, tranGetUserNameList: { - Access: accessAlwaysAllow, Name: "tranHandleGetUserNameList", Handler: HandleGetUserNameList, }, tranInviteNewChat: { - Access: accessOpenChat, - DenyMsg: "You are not allowed to request private chat.", Name: "tranInviteNewChat", Handler: HandleInviteNewChat, }, tranInviteToChat: { - Access: accessOpenChat, - DenyMsg: "You are not allowed to request private chat.", Name: "tranInviteToChat", Handler: HandleInviteToChat, }, tranJoinChat: { - Access: accessAlwaysAllow, Name: "tranJoinChat", Handler: HandleJoinChat, }, tranKeepAlive: { - Access: accessAlwaysAllow, Name: "tranKeepAlive", Handler: HandleKeepAlive, }, tranLeaveChat: { - Access: accessAlwaysAllow, Name: "tranJoinChat", Handler: HandleLeaveChat, }, tranListUsers: { - Access: accessAlwaysAllow, Name: "tranListUsers", Handler: HandleListUsers, }, tranMoveFile: { - Access: accessMoveFile, - DenyMsg: "You are not allowed to move files.", Name: "tranMoveFile", Handler: HandleMoveFile, }, tranNewFolder: { - Access: accessCreateFolder, - DenyMsg: "You are not allow to create folders.", Name: "tranNewFolder", Handler: HandleNewFolder, }, tranNewNewsCat: { - Access: accessNewsCreateCat, - DenyMsg: "You are not allowed to create news categories.", Name: "tranNewNewsCat", Handler: HandleNewNewsCat, }, tranNewNewsFldr: { - Access: accessNewsCreateFldr, - DenyMsg: "You are not allowed to create news folders.", Name: "tranNewNewsFldr", Handler: HandleNewNewsFldr, }, tranNewUser: { - Access: accessAlwaysAllow, Name: "tranNewUser", Handler: HandleNewUser, }, tranUpdateUser: { - Access: accessAlwaysAllow, Name: "tranUpdateUser", Handler: HandleUpdateUser, }, tranOldPostNews: { - Access: accessNewsPostArt, - DenyMsg: "You are not allowed to post news.", Name: "tranOldPostNews", Handler: HandleTranOldPostNews, }, tranPostNewsArt: { - Access: accessNewsPostArt, - DenyMsg: "You are not allowed to post news articles.", Name: "tranPostNewsArt", Handler: HandlePostNewsArt, }, tranRejectChatInvite: { - Access: accessAlwaysAllow, Name: "tranRejectChatInvite", Handler: HandleRejectChatInvite, }, tranSendInstantMsg: { - Access: accessAlwaysAllow, - // Access: accessSendPrivMsg, - // DenyMsg: "You are not allowed to send private messages", Name: "tranSendInstantMsg", Handler: HandleSendInstantMsg, RequiredFields: []requiredField{ @@ -249,12 +194,10 @@ var TransactionHandlers = map[uint16]TransactionType{ }, }, tranSetChatSubject: { - Access: accessAlwaysAllow, Name: "tranSetChatSubject", Handler: HandleSetChatSubject, }, tranMakeFileAlias: { - Access: accessAlwaysAllow, Name: "tranMakeFileAlias", Handler: HandleMakeAlias, RequiredFields: []requiredField{ @@ -264,34 +207,26 @@ var TransactionHandlers = map[uint16]TransactionType{ }, }, tranSetClientUserInfo: { - Access: accessAlwaysAllow, Name: "tranSetClientUserInfo", Handler: HandleSetClientUserInfo, }, tranSetFileInfo: { - Access: accessAlwaysAllow, Name: "tranSetFileInfo", Handler: HandleSetFileInfo, }, tranSetUser: { - Access: accessModifyUser, - DenyMsg: "You are not allowed to modify accounts.", Name: "tranSetUser", Handler: HandleSetUser, }, tranUploadFile: { - Access: accessAlwaysAllow, Name: "tranUploadFile", Handler: HandleUploadFile, }, tranUploadFldr: { - Access: accessAlwaysAllow, Name: "tranUploadFldr", Handler: HandleUploadFolder, }, tranUserBroadcast: { - Access: accessBroadcast, - DenyMsg: "You are not allowed to send broadcast messages.", Name: "tranUserBroadcast", Handler: HandleUserBroadcast, }, @@ -567,6 +502,10 @@ func HandleMoveFile(cc *ClientConn, t *Transaction) (res []Transaction, err erro } func HandleNewFolder(cc *ClientConn, t *Transaction) (res []Transaction, err error) { + if !authorize(cc.Account.Access, accessCreateFolder) { + res = append(res, cc.NewErrReply(t, "You are not allowed to create folders.")) + return res, err + } newFolderPath := cc.Server.Config.FileRoot folderName := string(t.GetField(fieldFileName).Data) @@ -602,6 +541,11 @@ func HandleNewFolder(cc *ClientConn, t *Transaction) (res []Transaction, err err } func HandleSetUser(cc *ClientConn, t *Transaction) (res []Transaction, err error) { + if !authorize(cc.Account.Access, accessModifyUser) { + res = append(res, cc.NewErrReply(t, "You are not allowed to modify accounts.")) + return res, err + } + login := DecodeUserString(t.GetField(fieldUserLogin).Data) userName := string(t.GetField(fieldUserName).Data) @@ -831,6 +775,11 @@ func HandleDeleteUser(cc *ClientConn, t *Transaction) (res []Transaction, err er // HandleUserBroadcast sends an Administrator Message to all connected clients of the server func HandleUserBroadcast(cc *ClientConn, t *Transaction) (res []Transaction, err error) { + if !authorize(cc.Account.Access, accessBroadcast) { + res = append(res, cc.NewErrReply(t, "You are not allowed to send broadcast messages.")) + return res, err + } + cc.sendAll( tranServerMsg, NewField(fieldData, t.GetField(tranGetMsgs).Data), @@ -853,6 +802,11 @@ func byteToInt(bytes []byte) (int, error) { } func HandleGetClientConnInfoText(cc *ClientConn, t *Transaction) (res []Transaction, err error) { + if !authorize(cc.Account.Access, accessGetClientInfo) { + res = append(res, cc.NewErrReply(t, "You are not allowed to get client info")) + return res, err + } + clientID, _ := byteToInt(t.GetField(fieldUserID).Data) clientConn := cc.Server.Clients[uint16(clientID)] @@ -899,7 +853,7 @@ None. clientConn.UserName, clientConn.Account.Name, clientConn.Account.Login, - clientConn.Connection.RemoteAddr().String(), + clientConn.RemoteAddr, activeDownloadList, ) template = strings.Replace(template, "\n", "\r", -1) @@ -974,6 +928,11 @@ __________________________________________________________` // Fields used in this request: // 101 Data func HandleTranOldPostNews(cc *ClientConn, t *Transaction) (res []Transaction, err error) { + if !authorize(cc.Account.Access, accessNewsPostArt) { + res = append(res, cc.NewErrReply(t, "You are not allowed to post news.")) + return res, err + } + cc.Server.flatNewsMux.Lock() defer cc.Server.flatNewsMux.Unlock() @@ -1009,6 +968,11 @@ func HandleTranOldPostNews(cc *ClientConn, t *Transaction) (res []Transaction, e } func HandleDisconnectUser(cc *ClientConn, t *Transaction) (res []Transaction, err error) { + if !authorize(cc.Account.Access, accessDisconUser) { + res = append(res, cc.NewErrReply(t, "You are not allowed to disconnect users.")) + return res, err + } + clientConn := cc.Server.Clients[binary.BigEndian.Uint16(t.GetField(fieldUserID).Data)] if authorize(clientConn.Account.Access, accessCannotBeDiscon) { @@ -1024,9 +988,14 @@ func HandleDisconnectUser(cc *ClientConn, t *Transaction) (res []Transaction, er return res, err } +// HandleGetNewsCatNameList returns a list of news categories for a path +// Fields used in the request: +// 325 News path (Optional) func HandleGetNewsCatNameList(cc *ClientConn, t *Transaction) (res []Transaction, err error) { - // Fields used in the request: - // 325 News path (Optional) + if !authorize(cc.Account.Access, accessNewsReadArt) { + res = append(res, cc.NewErrReply(t, "You are not allowed to read news.")) + return res, err + } newsPath := t.GetField(fieldNewsPath).Data cc.Server.Logger.Infow("NewsPath: ", "np", string(newsPath)) @@ -1058,6 +1027,11 @@ func HandleGetNewsCatNameList(cc *ClientConn, t *Transaction) (res []Transaction } func HandleNewNewsCat(cc *ClientConn, t *Transaction) (res []Transaction, err error) { + if !authorize(cc.Account.Access, accessNewsCreateCat) { + res = append(res, cc.NewErrReply(t, "You are not allowed to create news categories.")) + return res, err + } + name := string(t.GetField(fieldNewsCatName).Data) pathStrs := ReadNewsPath(t.GetField(fieldNewsPath).Data) @@ -1076,10 +1050,15 @@ func HandleNewNewsCat(cc *ClientConn, t *Transaction) (res []Transaction, err er return res, err } +// Fields used in the request: +// 322 News category name +// 325 News path func HandleNewNewsFldr(cc *ClientConn, t *Transaction) (res []Transaction, err error) { - // Fields used in the request: - // 322 News category name - // 325 News path + if !authorize(cc.Account.Access, accessNewsCreateFldr) { + res = append(res, cc.NewErrReply(t, "You are not allowed to create news folders.")) + return res, err + } + name := string(t.GetField(fieldFileName).Data) pathStrs := ReadNewsPath(t.GetField(fieldNewsPath).Data) @@ -1105,6 +1084,10 @@ func HandleNewNewsFldr(cc *ClientConn, t *Transaction) (res []Transaction, err e // Reply fields: // 321 News article list data Optional func HandleGetNewsArtNameList(cc *ClientConn, t *Transaction) (res []Transaction, err error) { + if !authorize(cc.Account.Access, accessNewsReadArt) { + res = append(res, cc.NewErrReply(t, "You are not allowed to read news.")) + return res, err + } pathStrs := ReadNewsPath(t.GetField(fieldNewsPath).Data) var cat NewsCategoryListData15 @@ -1122,6 +1105,11 @@ func HandleGetNewsArtNameList(cc *ClientConn, t *Transaction) (res []Transaction } func HandleGetNewsArtData(cc *ClientConn, t *Transaction) (res []Transaction, err error) { + if !authorize(cc.Account.Access, accessNewsReadArt) { + res = append(res, cc.NewErrReply(t, "You are not allowed to read news.")) + return res, err + } + // Request fields // 325 News fp // 326 News article ID @@ -1172,7 +1160,8 @@ func HandleGetNewsArtData(cc *ClientConn, t *Transaction) (res []Transaction, er } func HandleDelNewsItem(cc *ClientConn, t *Transaction) (res []Transaction, err error) { - // Access: News Delete Folder (37) or News Delete Category (35) + // Has multiple access flags: News Delete Folder (37) or News Delete Category (35) + // TODO: Implement pathStrs := ReadNewsPath(t.GetField(fieldNewsPath).Data) @@ -1203,6 +1192,11 @@ func HandleDelNewsItem(cc *ClientConn, t *Transaction) (res []Transaction, err e } func HandleDelNewsArt(cc *ClientConn, t *Transaction) (res []Transaction, err error) { + if !authorize(cc.Account.Access, accessNewsDeleteArt) { + res = append(res, cc.NewErrReply(t, "You are not allowed to delete news articles.")) + return res, err + } + // Request Fields // 325 News path // 326 News article ID @@ -1227,14 +1221,18 @@ func HandleDelNewsArt(cc *ClientConn, t *Transaction) (res []Transaction, err er return res, err } +// Request fields +// 325 News path +// 326 News article ID ID of the parent article? +// 328 News article title +// 334 News article flags +// 327 News article data flavor Currently “text/plain” +// 333 News article data func HandlePostNewsArt(cc *ClientConn, t *Transaction) (res []Transaction, err error) { - // Request fields - // 325 News path - // 326 News article ID ID of the parent article? - // 328 News article title - // 334 News article flags - // 327 News article data flavor Currently “text/plain” - // 333 News article data + if !authorize(cc.Account.Access, accessNewsPostArt) { + res = append(res, cc.NewErrReply(t, "You are not allowed to post news articles.")) + return res, err + } pathStrs := ReadNewsPath(t.GetField(fieldNewsPath).Data) cats := cc.Server.GetNewsCatByPath(pathStrs[:len(pathStrs)-1]) @@ -1347,7 +1345,9 @@ func HandleDownloadFile(cc *ClientConn, t *Transaction) (res []Transaction, err if resumeData != nil { var frd FileResumeData - frd.UnmarshalBinary(t.GetField(fieldFileResumeData).Data) + if err := frd.UnmarshalBinary(t.GetField(fieldFileResumeData).Data); err != nil { + return res, err + } ft.fileResumeData = &frd } @@ -1378,30 +1378,12 @@ func HandleDownloadFile(cc *ClientConn, t *Transaction) (res []Transaction, err } // Download all files from the specified folder and sub-folders -// response example -// -// 00 -// 01 -// 00 00 -// 00 00 00 11 -// 00 00 00 00 -// 00 00 00 18 -// 00 00 00 18 -// -// 00 03 -// -// 00 6c // transfer size -// 00 04 // len -// 00 0f d5 ae -// -// 00 dc // field Folder item count -// 00 02 // len -// 00 02 -// -// 00 6b // ref number -// 00 04 // len -// 00 03 64 b1 func HandleDownloadFolder(cc *ClientConn, t *Transaction) (res []Transaction, err error) { + if !authorize(cc.Account.Access, accessDownloadFile) { + res = append(res, cc.NewErrReply(t, "You are not allowed to download folders.")) + return res, err + } + transactionRef := cc.Server.NewTransactionRef() data := binary.BigEndian.Uint32(transactionRef) @@ -1661,6 +1643,11 @@ func HandleGetFileNameList(cc *ClientConn, t *Transaction) (res []Transaction, e // HandleInviteNewChat invites users to new private chat func HandleInviteNewChat(cc *ClientConn, t *Transaction) (res []Transaction, err error) { + if !authorize(cc.Account.Access, accessOpenChat) { + res = append(res, cc.NewErrReply(t, "You are not allowed to request private chat.")) + return res, err + } + // Client to Invite targetID := t.GetField(fieldUserID).Data newChatID := cc.Server.NewPrivateChat(cc) @@ -1689,6 +1676,11 @@ func HandleInviteNewChat(cc *ClientConn, t *Transaction) (res []Transaction, err } func HandleInviteToChat(cc *ClientConn, t *Transaction) (res []Transaction, err error) { + if !authorize(cc.Account.Access, accessOpenChat) { + res = append(res, cc.NewErrReply(t, "You are not allowed to request private chat.")) + return res, err + } + // Client to Invite targetID := t.GetField(fieldUserID).Data chatID := t.GetField(fieldChatID).Data diff --git a/hotline/transaction_handlers_test.go b/hotline/transaction_handlers_test.go index 6ff8cf1..d7ec098 100644 --- a/hotline/transaction_handlers_test.go +++ b/hotline/transaction_handlers_test.go @@ -695,10 +695,50 @@ func TestHandleNewFolder(t *testing.T) { wantRes []Transaction wantErr bool }{ + { + name: "without required permission", + setup: func() {}, + args: args{ + cc: &ClientConn{ + Account: &Account{ + Access: func() *[]byte { + var bits accessBitmap + access := bits[:] + return &access + }(), + }, + }, + t: NewTransaction( + accessCreateFolder, + &[]byte{0, 0}, + ), + }, + wantRes: []Transaction{ + { + Flags: 0x00, + IsReply: 0x01, + Type: []byte{0, 0x00}, + ID: []byte{0x9a, 0xcb, 0x04, 0x42}, + ErrorCode: []byte{0, 0, 0, 1}, + Fields: []Field{ + NewField(fieldError, []byte("You are not allowed to create folders.")), + }, + }, + }, + wantErr: false, + }, { name: "when path is nested", args: args{ cc: &ClientConn{ + Account: &Account{ + Access: func() *[]byte { + var bits accessBitmap + bits.Set(accessCreateFolder) + access := bits[:] + return &access + }(), + }, ID: &[]byte{0, 1}, Server: &Server{ Config: &Config{ @@ -739,6 +779,14 @@ func TestHandleNewFolder(t *testing.T) { name: "when path is not nested", args: args{ cc: &ClientConn{ + Account: &Account{ + Access: func() *[]byte { + var bits accessBitmap + bits.Set(accessCreateFolder) + access := bits[:] + return &access + }(), + }, ID: &[]byte{0, 1}, Server: &Server{ Config: &Config{ @@ -773,6 +821,14 @@ func TestHandleNewFolder(t *testing.T) { name: "when UnmarshalBinary returns an err", args: args{ cc: &ClientConn{ + Account: &Account{ + Access: func() *[]byte { + var bits accessBitmap + bits.Set(accessCreateFolder) + access := bits[:] + return &access + }(), + }, ID: &[]byte{0, 1}, Server: &Server{ Config: &Config{ @@ -801,6 +857,14 @@ func TestHandleNewFolder(t *testing.T) { name: "fieldFileName does not allow directory traversal", args: args{ cc: &ClientConn{ + Account: &Account{ + Access: func() *[]byte { + var bits accessBitmap + bits.Set(accessCreateFolder) + access := bits[:] + return &access + }(), + }, ID: &[]byte{0, 1}, Server: &Server{ Config: &Config{ @@ -834,6 +898,14 @@ func TestHandleNewFolder(t *testing.T) { name: "fieldFilePath does not allow directory traversal", args: args{ cc: &ClientConn{ + Account: &Account{ + Access: func() *[]byte { + var bits accessBitmap + bits.Set(accessCreateFolder) + access := bits[:] + return &access + }(), + }, ID: &[]byte{0, 1}, Server: &Server{ Config: &Config{ @@ -882,6 +954,7 @@ func TestHandleNewFolder(t *testing.T) { t.Errorf("HandleNewFolder() error = %v, wantErr %v", err, tt.wantErr) return } + if !tranAssertEqual(t, tt.wantRes, gotRes) { t.Errorf("HandleNewFolder() gotRes = %v, want %v", gotRes, tt.wantRes) } @@ -1907,3 +1980,159 @@ func TestHandleUpdateUser(t *testing.T) { }) } } + +func TestHandleDelNewsArt(t *testing.T) { + type args struct { + cc *ClientConn + t *Transaction + } + tests := []struct { + name string + args args + wantRes []Transaction + wantErr assert.ErrorAssertionFunc + }{ + { + name: "without required permission", + args: args{ + cc: &ClientConn{ + Account: &Account{ + Access: func() *[]byte { + var bits accessBitmap + access := bits[:] + return &access + }(), + }, + }, + t: NewTransaction( + tranDelNewsArt, + &[]byte{0, 0}, + ), + }, + wantRes: []Transaction{ + { + Flags: 0x00, + IsReply: 0x01, + Type: []byte{0, 0x00}, + ID: []byte{0x9a, 0xcb, 0x04, 0x42}, + ErrorCode: []byte{0, 0, 0, 1}, + Fields: []Field{ + NewField(fieldError, []byte("You are not allowed to delete news articles.")), + }, + }, + }, + wantErr: assert.NoError, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotRes, err := HandleDelNewsArt(tt.args.cc, tt.args.t) + if !tt.wantErr(t, err, fmt.Sprintf("HandleDelNewsArt(%v, %v)", tt.args.cc, tt.args.t)) { + return + } + tranAssertEqual(t, tt.wantRes, gotRes) + }) + } +} + +func TestHandleDisconnectUser(t *testing.T) { + type args struct { + cc *ClientConn + t *Transaction + } + tests := []struct { + name string + args args + wantRes []Transaction + wantErr assert.ErrorAssertionFunc + }{ + { + name: "without required permission", + args: args{ + cc: &ClientConn{ + Account: &Account{ + Access: func() *[]byte { + var bits accessBitmap + access := bits[:] + return &access + }(), + }, + }, + t: NewTransaction( + tranDelNewsArt, + &[]byte{0, 0}, + ), + }, + wantRes: []Transaction{ + { + Flags: 0x00, + IsReply: 0x01, + Type: []byte{0, 0x00}, + ID: []byte{0x9a, 0xcb, 0x04, 0x42}, + ErrorCode: []byte{0, 0, 0, 1}, + Fields: []Field{ + NewField(fieldError, []byte("You are not allowed to disconnect users.")), + }, + }, + }, + wantErr: assert.NoError, + }, + { + name: "when target user has 'cannot be disconnected' priv", + args: args{ + cc: &ClientConn{ + Server: &Server{ + Clients: map[uint16]*ClientConn{ + uint16(1): { + Account: &Account{ + Login: "unnamed", + Access: func() *[]byte { + var bits accessBitmap + bits.Set(accessCannotBeDiscon) + access := bits[:] + return &access + }(), + }, + }, + }, + }, + Account: &Account{ + Access: func() *[]byte { + var bits accessBitmap + bits.Set(accessDisconUser) + access := bits[:] + return &access + }(), + }, + }, + t: NewTransaction( + tranDelNewsArt, + &[]byte{0, 0}, + NewField(fieldUserID, []byte{0, 1}), + ), + }, + wantRes: []Transaction{ + { + Flags: 0x00, + IsReply: 0x01, + Type: []byte{0, 0x00}, + ID: []byte{0x9a, 0xcb, 0x04, 0x42}, + ErrorCode: []byte{0, 0, 0, 1}, + Fields: []Field{ + NewField(fieldError, []byte("unnamed is not allowed to be disconnected.")), + }, + }, + }, + wantErr: assert.NoError, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotRes, err := HandleDisconnectUser(tt.args.cc, tt.args.t) + if !tt.wantErr(t, err, fmt.Sprintf("HandleDisconnectUser(%v, %v)", tt.args.cc, tt.args.t)) { + return + } + tranAssertEqual(t, tt.wantRes, gotRes) + }) + } +} diff --git a/hotline/ui.go b/hotline/ui.go index 168bf3a..b9e45dd 100644 --- a/hotline/ui.go +++ b/hotline/ui.go @@ -2,7 +2,6 @@ package hotline import ( "fmt" - "github.com/davecgh/go-spew/spew" "github.com/gdamore/tcell/v2" "github.com/rivo/tview" "gopkg.in/yaml.v3" @@ -108,7 +107,7 @@ func (ui *UI) showBookmarks() *tview.List { func (ui *UI) getTrackerList() *tview.List { listing, err := GetListing(ui.HLClient.pref.Tracker) if err != nil { - spew.Dump(err) + // TODO } list := tview.NewList()