Implement access controls for threaded news item deletion

4 files changed, 198 insertions, 23 deletions

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)
+		})
+	}
+}