From: Jeff Halter Date: Sun, 29 May 2022 16:34:24 +0000 (-0700) Subject: Refactor and cleanup X-Git-Url: https://git.r.bdr.sh/rbdr/mobius/commitdiff_plain/003a743e6767b3041c3a8321566c3586d73b399a Refactor and cleanup --- diff --git a/hotline/client_conn.go b/hotline/client_conn.go index ce9d7f1..4a471b3 100644 --- a/hotline/client_conn.go +++ b/hotline/client_conn.go @@ -157,15 +157,15 @@ func (cc ClientConn) Disconnect() { delete(cc.Server.Clients, binary.BigEndian.Uint16(*cc.ID)) - cc.NotifyOthers(*NewTransaction(tranNotifyDeleteUser, nil, NewField(fieldUserID, *cc.ID))) + 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()) } } -// NotifyOthers sends transaction t to other clients connected to the server -func (cc ClientConn) NotifyOthers(t Transaction) { +// notifyOthers sends transaction t to other clients connected to the server +func (cc ClientConn) notifyOthers(t Transaction) { for _, c := range sortedClients(cc.Server.Clients) { if c.ID != cc.ID && c.Agreed { t.clientID = c.ID diff --git a/hotline/file_store.go b/hotline/file_store.go index f528c36..e9d083f 100644 --- a/hotline/file_store.go +++ b/hotline/file_store.go @@ -12,7 +12,7 @@ type FileStore interface { Stat(name string) (os.FileInfo, error) Open(name string) (*os.File, error) Symlink(oldname, newname string) error - + Remove(name string) error // TODO: implement these // Rename(oldpath string, newpath string) error // RemoveAll(path string) error @@ -36,6 +36,10 @@ func (fs *OSFileStore) Symlink(oldname, newname string) error { return os.Symlink(oldname, newname) } +func (fs *OSFileStore) Remove(name string) error { + return os.Remove(name) +} + type MockFileStore struct { mock.Mock } @@ -63,3 +67,8 @@ func (mfs *MockFileStore) Symlink(oldname, newname string) error { args := mfs.Called(oldname, newname) return args.Error(0) } + +func (mfs *MockFileStore) Remove(name string) error { + args := mfs.Called(name) + return args.Error(0) +} diff --git a/hotline/server.go b/hotline/server.go index 3a58893..7b409d5 100644 --- a/hotline/server.go +++ b/hotline/server.go @@ -365,7 +365,7 @@ func (s *Server) DeleteUser(login string) error { delete(s.Accounts, login) - return os.Remove(s.ConfigDir + "Users/" + login + ".yaml") + return FS.Remove(s.ConfigDir + "Users/" + login + ".yaml") } func (s *Server) connectedUsers() []Field { @@ -538,9 +538,16 @@ func (s *Server) handleNewConnection(conn net.Conn) error { // assume simplified hotline v1.2.3 login flow that does not require agreement if *c.Version == nil { c.Agreed = true - if _, err := c.notifyNewUserHasJoined(); err != nil { - return err - } + + c.notifyOthers( + *NewTransaction( + tranNotifyChangeUser, nil, + NewField(fieldUserName, c.UserName), + NewField(fieldUserID, *c.ID), + NewField(fieldUserIconID, *c.Icon), + NewField(fieldUserFlags, *c.Flags), + ), + ) } c.Server.Stats.LoginCount += 1 diff --git a/hotline/transaction_handlers.go b/hotline/transaction_handlers.go index 91a4c0d..270f73e 100644 --- a/hotline/transaction_handlers.go +++ b/hotline/transaction_handlers.go @@ -45,13 +45,10 @@ var TransactionHandlers = map[uint16]TransactionType{ Name: "tranNotifyDeleteUser", }, tranAgreed: { - Access: accessAlwaysAllow, Name: "tranAgreed", Handler: HandleTranAgreed, }, tranChatSend: { - Access: accessSendChat, - DenyMsg: "You are not allowed to participate in chat.", Handler: HandleChatSend, Name: "tranChatSend", RequiredFields: []requiredField{ @@ -68,20 +65,16 @@ var TransactionHandlers = map[uint16]TransactionType{ 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: accessDeleteUser, - DenyMsg: "You are not allowed to delete accounts.", Name: "tranDeleteUser", Handler: HandleDeleteUser, }, @@ -110,12 +103,10 @@ var TransactionHandlers = map[uint16]TransactionType{ Handler: HandleGetClientConnInfoText, }, tranGetFileInfo: { - Access: accessAlwaysAllow, Name: "tranGetFileInfo", Handler: HandleGetFileInfo, }, tranGetFileNameList: { - Access: accessAlwaysAllow, Name: "tranGetFileNameList", Handler: HandleGetFileNameList, }, @@ -144,13 +135,11 @@ var TransactionHandlers = map[uint16]TransactionType{ Handler: HandleGetNewsCatNameList, }, tranGetUser: { - Access: accessOpenUser, DenyMsg: "You are not allowed to view accounts.", Name: "tranGetUser", Handler: HandleGetUser, }, tranGetUserNameList: { - Access: accessAlwaysAllow, Name: "tranHandleGetUserNameList", Handler: HandleGetUserNameList, }, @@ -167,17 +156,14 @@ var TransactionHandlers = map[uint16]TransactionType{ Handler: HandleInviteToChat, }, tranJoinChat: { - Access: accessAlwaysAllow, Name: "tranJoinChat", Handler: HandleJoinChat, }, tranKeepAlive: { - Access: accessAlwaysAllow, Name: "tranKeepAlive", Handler: HandleKeepAlive, }, tranLeaveChat: { - Access: accessAlwaysAllow, Name: "tranJoinChat", Handler: HandleLeaveChat, }, @@ -231,7 +217,6 @@ var TransactionHandlers = map[uint16]TransactionType{ Handler: HandlePostNewsArt, }, tranRejectChatInvite: { - Access: accessAlwaysAllow, Name: "tranRejectChatInvite", Handler: HandleRejectChatInvite, }, @@ -252,12 +237,10 @@ var TransactionHandlers = map[uint16]TransactionType{ }, }, tranSetChatSubject: { - Access: accessAlwaysAllow, Name: "tranSetChatSubject", Handler: HandleSetChatSubject, }, tranMakeFileAlias: { - Access: accessAlwaysAllow, Name: "tranMakeFileAlias", Handler: HandleMakeAlias, RequiredFields: []requiredField{ @@ -267,12 +250,10 @@ var TransactionHandlers = map[uint16]TransactionType{ }, }, tranSetClientUserInfo: { - Access: accessAlwaysAllow, Name: "tranSetClientUserInfo", Handler: HandleSetClientUserInfo, }, tranSetFileInfo: { - Access: accessAlwaysAllow, // granular access is in the handler Name: "tranSetFileInfo", Handler: HandleSetFileInfo, }, @@ -283,13 +264,10 @@ var TransactionHandlers = map[uint16]TransactionType{ Handler: HandleSetUser, }, tranUploadFile: { - Access: accessAlwaysAllow, - DenyMsg: "You are not allowed to upload files.", Name: "tranUploadFile", Handler: HandleUploadFile, }, tranUploadFldr: { - Access: accessAlwaysAllow, // TODO: what should this be? Name: "tranUploadFldr", Handler: HandleUploadFolder, }, @@ -302,6 +280,11 @@ var TransactionHandlers = map[uint16]TransactionType{ } func HandleChatSend(cc *ClientConn, t *Transaction) (res []Transaction, err error) { + if !authorize(cc.Account.Access, accessSendChat) { + res = append(res, cc.NewErrReply(t, "You are not allowed to participate in chat.")) + return res, err + } + // Truncate long usernames trunc := fmt.Sprintf("%13s", cc.UserName) formattedMsg := fmt.Sprintf("\r%.14s: %s", trunc, t.GetField(fieldData).Data) @@ -527,8 +510,8 @@ func HandleMoveFile(cc *ClientConn, t *Transaction) (res []Transaction, err erro cc.Server.Logger.Debugw("Move file", "src", filePath+"/"+fileName, "dst", fileNewPath+"/"+fileName) - path := filePath + "/" + fileName - fi, err := os.Stat(path) + fp := filePath + "/" + fileName + fi, err := os.Stat(fp) if err != nil { return res, err } @@ -649,15 +632,16 @@ func HandleSetUser(cc *ClientConn, t *Transaction) (res []Transaction, err error } } - // TODO: If we have just promoted a connected user to admin, notify - // connected clients to turn the user red - res = append(res, cc.NewReply(t)) return res, err } func HandleGetUser(cc *ClientConn, t *Transaction) (res []Transaction, err error) { - // userLogin := string(t.GetField(fieldUserLogin).Data) + if !authorize(cc.Account.Access, accessOpenUser) { + res = append(res, cc.NewErrReply(t, "You are not allowed to view accounts.")) + return res, err + } + account := cc.Server.Accounts[string(t.GetField(fieldUserLogin).Data)] if account == nil { errorT := cc.NewErrReply(t, "Account does not exist.") @@ -711,6 +695,11 @@ func HandleNewUser(cc *ClientConn, t *Transaction) (res []Transaction, err error } func HandleDeleteUser(cc *ClientConn, t *Transaction) (res []Transaction, err error) { + if !authorize(cc.Account.Access, accessDeleteUser) { + res = append(res, cc.NewErrReply(t, "You are not allowed to delete accounts.")) + return res, err + } + // TODO: Handle case where account doesn't exist; e.g. delete race condition login := DecodeUserString(t.GetField(fieldUserLogin).Data) @@ -810,21 +799,6 @@ func HandleGetUserNameList(cc *ClientConn, t *Transaction) (res []Transaction, e return res, err } -func (cc *ClientConn) notifyNewUserHasJoined() (res []Transaction, err error) { - // Notify other ccs that a new user has connected - cc.NotifyOthers( - *NewTransaction( - tranNotifyChangeUser, nil, - NewField(fieldUserName, cc.UserName), - NewField(fieldUserID, *cc.ID), - NewField(fieldUserIconID, *cc.Icon), - NewField(fieldUserFlags, *cc.Flags), - ), - ) - - return res, nil -} - func HandleTranAgreed(cc *ClientConn, t *Transaction) (res []Transaction, err error) { cc.Agreed = true cc.UserName = t.GetField(fieldUserName).Data @@ -854,7 +828,15 @@ func HandleTranAgreed(cc *ClientConn, t *Transaction) (res []Transaction, err er cc.AutoReply = []byte{} } - _, _ = cc.notifyNewUserHasJoined() + cc.notifyOthers( + *NewTransaction( + tranNotifyChangeUser, nil, + NewField(fieldUserName, cc.UserName), + NewField(fieldUserID, *cc.ID), + NewField(fieldUserIconID, *cc.Icon), + NewField(fieldUserFlags, *cc.Flags), + ), + ) res = append(res, cc.NewReply(t)) @@ -1010,9 +992,9 @@ func HandleGetNewsArtNameList(cc *ClientConn, t *Transaction) (res []Transaction var cat NewsCategoryListData15 cats := cc.Server.ThreadedNews.Categories - for _, path := range pathStrs { - cat = cats[path] - cats = cats[path].SubCats + for _, fp := range pathStrs { + cat = cats[fp] + cats = cats[fp].SubCats } nald := cat.GetNewsArtListData() @@ -1023,7 +1005,7 @@ func HandleGetNewsArtNameList(cc *ClientConn, t *Transaction) (res []Transaction func HandleGetNewsArtData(cc *ClientConn, t *Transaction) (res []Transaction, err error) { // Request fields - // 325 News path + // 325 News fp // 326 News article ID // 327 News article data flavor @@ -1032,9 +1014,9 @@ func HandleGetNewsArtData(cc *ClientConn, t *Transaction) (res []Transaction, er var cat NewsCategoryListData15 cats := cc.Server.ThreadedNews.Categories - for _, path := range pathStrs { - cat = cats[path] - cats = cats[path].SubCats + for _, fp := range pathStrs { + cat = cats[fp] + cats = cats[fp].SubCats } newsArtID := t.GetField(fieldNewsArtID).Data @@ -1351,13 +1333,6 @@ func HandleUploadFile(cc *ClientConn, t *Transaction) (res []Transaction, err er return res, err } -// User options -const ( - refusePM = 0 - refuseChat = 1 - autoResponse = 2 -) - func HandleSetClientUserInfo(cc *ClientConn, t *Transaction) (res []Transaction, err error) { var icon []byte if len(t.GetField(fieldUserIconID).Data) == 4 { diff --git a/hotline/user.go b/hotline/user.go index 00fe98c..ff4006a 100644 --- a/hotline/user.go +++ b/hotline/user.go @@ -14,6 +14,13 @@ const ( userFLagRefusePChat = 3 // User refuses private chat ) +// fieldOptions flags are sent from v1.5+ clients as part of tranAgreed +const ( + refusePM = 0 // User has "Refuse private messages" pref set + refuseChat = 1 // User has "Refuse private chat" pref set + autoResponse = 2 // User has "Automatic response" pref set +) + type User struct { ID []byte // Size 2 Icon []byte // Size 2