From: Jeff Halter Date: Wed, 29 Jun 2022 04:49:48 +0000 (-0700) Subject: Implement access controls for threaded news item deletion X-Git-Url: https://git.r.bdr.sh/rbdr/mobius/commitdiff_plain/8eb43f95a6f11b6c256ff339399e9479898b4380?ds=sidebyside;hp=-c Implement access controls for threaded news item deletion --- 8eb43f95a6f11b6c256ff339399e9479898b4380 diff --git a/hotline/news.go b/hotline/news.go index 77c6696..e25a2c8 100644 --- a/hotline/news.go +++ b/hotline/news.go @@ -202,6 +202,7 @@ func (newscat *NewsCategoryListData15) nameLen() []byte { return []byte{uint8(len(newscat.Name))} } +// TODO: re-implement as bufio.Scanner interface func ReadNewsPath(newsPath []byte) []string { if len(newsPath) == 0 { return []string{} diff --git a/hotline/server.go b/hotline/server.go index ab61e81..392c8f3 100644 --- a/hotline/server.go +++ b/hotline/server.go @@ -43,12 +43,10 @@ const ( var nostalgiaVersion = []byte{0, 0, 2, 0x2c} // version ID used by the Nostalgia client type Server struct { - Port int - Accounts map[string]*Account - Agreement []byte - Clients map[uint16]*ClientConn - ThreadedNews *ThreadedNews - + Port int + Accounts map[string]*Account + Agreement []byte + Clients map[uint16]*ClientConn fileTransfers map[[4]byte]*FileTransfer Config *Config @@ -64,6 +62,9 @@ type Server struct { outbox chan Transaction mux sync.Mutex + threadedNewsMux sync.Mutex + ThreadedNews *ThreadedNews + flatNewsMux sync.Mutex FlatNews []byte @@ -342,14 +343,14 @@ func (s *Server) writeBanList() error { } func (s *Server) writeThreadedNews() error { - s.mux.Lock() - defer s.mux.Unlock() + s.threadedNewsMux.Lock() + defer s.threadedNewsMux.Unlock() out, err := yaml.Marshal(s.ThreadedNews) if err != nil { return err } - err = ioutil.WriteFile( + err = s.FS.WriteFile( filepath.Join(s.ConfigDir, "ThreadedNews.yaml"), out, 0666, diff --git a/hotline/transaction_handlers.go b/hotline/transaction_handlers.go index 9b25ef0..8e2ef6b 100644 --- a/hotline/transaction_handlers.go +++ b/hotline/transaction_handlers.go @@ -1244,18 +1244,15 @@ func HandleGetNewsArtData(cc *ClientConn, t *Transaction) (res []Transaction, er return res, err } +// HandleDelNewsItem deletes an existing threaded news folder or category from the server. +// Fields used in the request: +// 325 News path +// Fields used in the reply: +// None func HandleDelNewsItem(cc *ClientConn, t *Transaction) (res []Transaction, err error) { - // Has multiple access flags: News Delete Folder (37) or News Delete Category (35) - // TODO: Implement - pathStrs := ReadNewsPath(t.GetField(fieldNewsPath).Data) - // TODO: determine if path is a Folder (Bundle) or Category and check for permission - - cc.logger.Infof("DelNewsItem %v", pathStrs) - cats := cc.Server.ThreadedNews.Categories - delName := pathStrs[len(pathStrs)-1] if len(pathStrs) > 1 { for _, fp := range pathStrs[0 : len(pathStrs)-1] { @@ -1263,17 +1260,23 @@ func HandleDelNewsItem(cc *ClientConn, t *Transaction) (res []Transaction, err e } } + if bytes.Compare(cats[delName].Type, []byte{0, 3}) == 0 { + if !cc.Authorize(accessNewsDeleteCat) { + return append(res, cc.NewErrReply(t, "You are not allowed to delete news categories.")), nil + } + } else { + if !cc.Authorize(accessNewsDeleteFldr) { + return append(res, cc.NewErrReply(t, "You are not allowed to delete news folders.")), nil + } + } + delete(cats, delName) - err = cc.Server.writeThreadedNews() - if err != nil { + if err := cc.Server.writeThreadedNews(); err != nil { return res, err } - // Reply params: none - res = append(res, cc.NewReply(t)) - - return res, err + return append(res, cc.NewReply(t)), nil } func HandleDelNewsArt(cc *ClientConn, t *Transaction) (res []Transaction, err error) { diff --git a/hotline/transaction_handlers_test.go b/hotline/transaction_handlers_test.go index b5e0bac..893a537 100644 --- a/hotline/transaction_handlers_test.go +++ b/hotline/transaction_handlers_test.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "io/fs" "math/rand" "os" @@ -2959,3 +2960,172 @@ func TestHandleSetClientUserInfo(t *testing.T) { }) } } + +func TestHandleDelNewsItem(t *testing.T) { + type args struct { + cc *ClientConn + t *Transaction + } + tests := []struct { + name string + args args + wantRes []Transaction + wantErr assert.ErrorAssertionFunc + }{ + { + name: "when user does not have permission to delete a news category", + args: args{ + cc: &ClientConn{ + Account: &Account{ + Access: accessBitmap{}, + }, + ID: &[]byte{0, 1}, + Server: &Server{ + ThreadedNews: &ThreadedNews{Categories: map[string]NewsCategoryListData15{ + "test": { + Type: []byte{0, 3}, + Count: nil, + NameSize: 0, + Name: "zz", + }, + }}, + }, + }, + t: NewTransaction( + tranDelNewsItem, nil, + NewField(fieldNewsPath, + []byte{ + 0, 1, + 0, 0, + 4, + 0x74, 0x65, 0x73, 0x74, + }, + ), + ), + }, + wantRes: []Transaction{ + { + clientID: &[]byte{0, 1}, + Flags: 0x00, + IsReply: 0x01, + Type: []byte{0, 0x00}, + ID: []byte{0, 0, 0, 0}, + ErrorCode: []byte{0, 0, 0, 1}, + Fields: []Field{ + NewField(fieldError, []byte("You are not allowed to delete news categories.")), + }, + }, + }, + wantErr: assert.NoError, + }, + { + name: "when user does not have permission to delete a news folder", + args: args{ + cc: &ClientConn{ + Account: &Account{ + Access: accessBitmap{}, + }, + ID: &[]byte{0, 1}, + Server: &Server{ + ThreadedNews: &ThreadedNews{Categories: map[string]NewsCategoryListData15{ + "testcat": { + Type: []byte{0, 2}, + Count: nil, + NameSize: 0, + Name: "test", + }, + }}, + }, + }, + t: NewTransaction( + tranDelNewsItem, nil, + NewField(fieldNewsPath, + []byte{ + 0, 1, + 0, 0, + 4, + 0x74, 0x65, 0x73, 0x74, + }, + ), + ), + }, + wantRes: []Transaction{ + { + clientID: &[]byte{0, 1}, + Flags: 0x00, + IsReply: 0x01, + Type: []byte{0, 0x00}, + ID: []byte{0, 0, 0, 0}, + ErrorCode: []byte{0, 0, 0, 1}, + Fields: []Field{ + NewField(fieldError, []byte("You are not allowed to delete news folders.")), + }, + }, + }, + wantErr: assert.NoError, + }, + { + name: "when user deletes a news folder", + args: args{ + cc: &ClientConn{ + Account: &Account{ + Access: func() accessBitmap { + var bits accessBitmap + bits.Set(accessNewsDeleteFldr) + return bits + }(), + }, + ID: &[]byte{0, 1}, + Server: &Server{ + ConfigDir: "/fakeConfigRoot", + FS: func() *MockFileStore { + mfs := &MockFileStore{} + mfs.On("WriteFile", "/fakeConfigRoot/ThreadedNews.yaml", mock.Anything, mock.Anything).Return(nil, os.ErrNotExist) + return mfs + }(), + ThreadedNews: &ThreadedNews{Categories: map[string]NewsCategoryListData15{ + "testcat": { + Type: []byte{0, 2}, + Count: nil, + NameSize: 0, + Name: "test", + }, + }}, + }, + }, + t: NewTransaction( + tranDelNewsItem, nil, + NewField(fieldNewsPath, + []byte{ + 0, 1, + 0, 0, + 4, + 0x74, 0x65, 0x73, 0x74, + }, + ), + ), + }, + wantRes: []Transaction{ + { + clientID: &[]byte{0, 1}, + Flags: 0x00, + IsReply: 0x01, + Type: []byte{0x01, 0x7c}, + ID: []byte{0, 0, 0, 0}, + ErrorCode: []byte{0, 0, 0, 0}, + Fields: []Field{}, + }, + }, + wantErr: assert.NoError, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotRes, err := HandleDelNewsItem(tt.args.cc, tt.args.t) + if !tt.wantErr(t, err, fmt.Sprintf("HandleDelNewsItem(%v, %v)", tt.args.cc, tt.args.t)) { + return + } + tranAssertEqual(t, tt.wantRes, gotRes) + }) + } +}