Implemented antispam via feedback replyto ratelimiting (#341)

* Implemented feedback replyto ratelimiting and a different field for the name

* fixed issues in the tests

server/api/tumdev/campus_backend.proto

@@ -472,6 +472,8 @@ message CreateFeedbackRequest {
   Recipient recipient = 1;
   // the email address of the user
   string from_email = 2;
+  // how the person wants to be called
+  string from_name = 8;
   // The actual message
   string message = 3;
   // Optional location which the user can choose (data protection) to attach or not

server/api/tumdev/campus_backend.swagger.json

@@ -493,6 +493,13 @@
             "required": false,
             "type": "string"
           },
+          {
+            "name": "fromName",
+            "description": "how the person wants to be called",
+            "in": "query",
+            "required": false,
+            "type": "string"
+          },
           {
             "name": "message",
             "description": "The actual message",

server/backend/cron/feedback_email.go

@@ -62,11 +62,13 @@ func messageWithHeaders(feedback *model.Feedback) *gomail.Message {
 	if feedback.Recipient != "" {
 		m.SetHeader("To", feedback.Recipient)
 	} else {
-		m.SetHeader("To", "app@tum.de") // should not ever happen as checked in the api
+		m.SetAddressHeader("To", "app@tum.de", "TCA Support") // should not ever happen as checked in the api
 	}
 	// ReplyTo
-	if feedback.ReplyTo.Valid {
-		m.SetHeader("Reply-To", feedback.ReplyTo.String)
+	if feedback.ReplyToName.Valid && feedback.ReplyToEmail.Valid {
+		m.SetAddressHeader("Reply-To", feedback.ReplyToEmail.String, feedback.ReplyToName.String)
+	} else if feedback.ReplyToEmail.Valid {
+		m.SetHeader("Reply-To", feedback.ReplyToEmail.String)
 	}
 	// Timestamp
 	if feedback.Timestamp.Valid {

server/backend/cron/feedback_email_test.go

@@ -23,31 +23,33 @@ func TestIterate(t *testing.T) {
 
 func fullFeedback() *model.Feedback {
 	return &model.Feedback{
-		EmailId:    "magic-id",
-		Recipient:  "tca",
-		ReplyTo:    null.StringFrom("test@example.de"),
-		Feedback:   "This is a Test",
-		ImageCount: 1,
-		Latitude:   null.FloatFrom(0),
-		Longitude:  null.FloatFrom(0),
-		AppVersion: null.StringFrom("TCA 10.2"),
-		OsVersion:  null.StringFrom("Android 10.0"),
-		Timestamp:  null.TimeFrom(time.Now()),
+		EmailId:      "magic-id",
+		Recipient:    "tca",
+		ReplyToEmail: null.StringFrom("test@example.de"),
+		ReplyToName:  null.StringFrom("Erika Mustermann"),
+		Feedback:     "This is a Test",
+		ImageCount:   1,
+		Latitude:     null.FloatFrom(0),
+		Longitude:    null.FloatFrom(0),
+		AppVersion:   null.StringFrom("TCA 10.2"),
+		OsVersion:    null.StringFrom("Android 10.0"),
+		Timestamp:    null.TimeFrom(time.Now()),
 	}
 }
 
 func emptyFeedback() *model.Feedback {
 	return &model.Feedback{
-		EmailId:    "",
-		Recipient:  "",
-		ReplyTo:    null.String{},
-		Feedback:   "",
-		ImageCount: 0,
-		Latitude:   null.Float{},
-		Longitude:  null.Float{},
-		AppVersion: null.String{},
-		OsVersion:  null.String{},
-		Timestamp:  null.Time{},
+		EmailId:      "",
+		Recipient:    "",
+		ReplyToEmail: null.String{},
+		ReplyToName:  null.String{},
+		Feedback:     "",
+		ImageCount:   0,
+		Latitude:     null.Float{},
+		Longitude:    null.Float{},
+		AppVersion:   null.String{},
+		OsVersion:    null.String{},
+		Timestamp:    null.Time{},
 	}
 }
 
@@ -58,7 +60,7 @@ func TestHeaderInstantiationWithFullFeedback(t *testing.T) {
 	m := messageWithHeaders(fb)
 	assert.Equal(t, []string{`"TUM Campus App" <from@example.de>`}, m.GetHeader("From"))
 	assert.Equal(t, []string{fb.Recipient}, m.GetHeader("To"))
-	assert.Equal(t, []string{"test@example.de"}, m.GetHeader("Reply-To"))
+	assert.Equal(t, []string{"\"Erika Mustermann\" <test@example.de>"}, m.GetHeader("Reply-To"))
 	assert.Equal(t, []string{fb.Timestamp.Time.Format(time.RFC1123Z)}, m.GetHeader("Date"))
 	assert.Equal(t, []string{"Feedback via the TUM Campus App"}, m.GetHeader("Subject"))
 }
@@ -68,7 +70,7 @@ func TestHeaderInstantiationWithEmptyFeedback(t *testing.T) {
 	require.NoError(t, os.Setenv("SMTP_FROM", "from@example.de"))
 	m := messageWithHeaders(emptyFeedback())
 	assert.Equal(t, []string{`"TUM Campus App" <from@example.de>`}, m.GetHeader("From"))
-	assert.Equal(t, []string{"app@tum.de"}, m.GetHeader("To"))
+	assert.Equal(t, []string{"\"TCA Support\" <app@tum.de>"}, m.GetHeader("To"))
 	assert.Equal(t, []string(nil), m.GetHeader("Reply-To"))
 	// Date is set to now in messageWithHeaders => checking that this is actually now is a bit tricker
 	dates := m.GetHeader("Date")

server/backend/feedback.go

@@ -9,6 +9,7 @@ import (
 	"path"
 	"slices"
 	"strings"
+	"time"
 
 	pb "github.com/TUM-Dev/Campus-Backend/server/api/tumdev"
 	"github.com/TUM-Dev/Campus-Backend/server/backend/cron"
@@ -62,10 +63,18 @@ func (s *CampusServer) CreateFeedback(stream pb.Campus_CreateFeedbackServer) err
 	if feedback.Feedback == "" && feedback.ImageCount == 0 {
 		return status.Error(codes.InvalidArgument, "Please attach an image or feedback for us")
 	}
+	if feedback.ReplyToEmail.Valid {
+		now := time.Now()
+		fiveMinutesAgo := now.Add(time.Minute * -5).Unix()
+		lastFeedback, feedbackExisted := s.feedbackEmailLastReuestAt.LoadOrStore(feedback.ReplyToEmail.String, now.Unix())
+		if feedbackExisted && lastFeedback.(int64) >= fiveMinutesAgo {
+			return status.Error(codes.ResourceExhausted, fmt.Sprintf("You have already send a feedback recently. Please wait %d seconds", lastFeedback.(int64)-fiveMinutesAgo))
+		}
+	}
 	// save feedback to db
 	if err := s.db.WithContext(stream.Context()).Transaction(func(tx *gorm.DB) error {
 		var existingFeeedbackCnt int64
-		if err := tx.Model(&feedback).Where("receiver=? AND reply_to=? AND feedback=? AND app_version=?", feedback.Recipient, feedback.ReplyTo, feedback.Feedback, feedback.AppVersion).Count(&existingFeeedbackCnt).Error; err != nil {
+		if err := tx.Model(&feedback).Where("receiver=? AND reply_to_email=? AND feedback=? AND app_version=?", feedback.Recipient, feedback.ReplyToEmail, feedback.Feedback, feedback.AppVersion).Count(&existingFeeedbackCnt).Error; err != nil {
 			return err
 		}
 		if existingFeeedbackCnt != 0 {
@@ -165,7 +174,10 @@ func mergeFeedback(feedback *model.Feedback, req *pb.CreateFeedbackRequest) {
 		feedback.Feedback = req.Message
 	}
 	if req.FromEmail != "" {
-		feedback.ReplyTo = null.StringFrom(req.FromEmail)
+		feedback.ReplyToEmail = null.StringFrom(req.FromEmail)
+	}
+	if req.FromName != "" {
+		feedback.ReplyToName = null.StringFrom(req.FromEmail)
 	}
 }
 

server/backend/feedback_test.go

@@ -10,6 +10,7 @@ import (
 	"os"
 	"path"
 	"regexp"
+	"sync"
 	"testing"
 	"time"
 
@@ -99,10 +100,10 @@ func (s *FeedbackSuite) Test_CreateFeedback_OneFile() {
 		}
 	}(cron.StorageDir)
 
-	server := CampusServer{db: s.DB}
+	server := CampusServer{db: s.DB, feedbackEmailLastReuestAt: &sync.Map{}}
 	s.mock.ExpectBegin()
 	returnedTime := time.Now()
-	s.mock.ExpectQuery(regexp.QuoteMeta("SELECT count(*) FROM `feedback` WHERE receiver=? AND reply_to=? AND feedback=? AND app_version=?")).
+	s.mock.ExpectQuery(regexp.QuoteMeta("SELECT count(*) FROM `feedback` WHERE receiver=? AND reply_to_email=? AND feedback=? AND app_version=?")).
 		WithArgs("app@tum.de", "testing@example.com", "Hello with image", nil).
 		WillReturnRows(sqlmock.NewRows([]string{"count(*)"}))
 	s.mock.ExpectQuery(regexp.QuoteMeta("INSERT INTO `files` (`name`,`path`,`downloads`,`downloaded`) VALUES (?,?,?,?) RETURNING `url`,`file`")).
@@ -111,8 +112,8 @@ func (s *FeedbackSuite) Test_CreateFeedback_OneFile() {
 	s.mock.ExpectQuery(regexp.QuoteMeta("INSERT INTO `files` (`name`,`path`,`downloads`,`downloaded`) VALUES (?,?,?,?) RETURNING `url`,`file`")).
 		WithArgs("1.png", sqlmock.AnyArg(), 1, true).
 		WillReturnRows(sqlmock.NewRows([]string{"url", "file"}).AddRow(nil, 1))
-	s.mock.ExpectQuery(regexp.QuoteMeta("INSERT INTO `feedback` (`image_count`,`email_id`,`receiver`,`reply_to`,`feedback`,`latitude`,`longitude`,`os_version`,`app_version`,`processed`) VALUES (?,?,?,?,?,?,?,?,?,?) RETURNING `timestamp`,`id`")).
-		WithArgs(2, sqlmock.AnyArg(), "app@tum.de", "testing@example.com", "Hello with image", nil, nil, nil, nil, false).
+	s.mock.ExpectQuery(regexp.QuoteMeta("INSERT INTO `feedback` (`image_count`,`email_id`,`receiver`,`reply_to_email`,`reply_to_name`,`feedback`,`latitude`,`longitude`,`os_version`,`app_version`,`processed`) VALUES (?,?,?,?,?,?,?,?,?,?,?) RETURNING `timestamp`,`id`")).
+		WithArgs(2, sqlmock.AnyArg(), "app@tum.de", "testing@example.com", nil, "Hello with image", nil, nil, nil, nil, false).
 		WillReturnRows(sqlmock.NewRows([]string{"timestamp", "id"}).AddRow(returnedTime, 1))
 	s.mock.ExpectCommit()
 
@@ -146,13 +147,13 @@ func expectFileMatches(t *testing.T, file os.DirEntry, name string, returnedTime
 func (s *FeedbackSuite) Test_CreateFeedback_NoImage() {
 	cron.StorageDir = "test_no_image/"
 
-	server := CampusServer{db: s.DB}
+	server := CampusServer{db: s.DB, feedbackEmailLastReuestAt: &sync.Map{}}
 	s.mock.ExpectBegin()
-	s.mock.ExpectQuery(regexp.QuoteMeta("SELECT count(*) FROM `feedback` WHERE receiver=? AND reply_to=? AND feedback=? AND app_version=?")).
+	s.mock.ExpectQuery(regexp.QuoteMeta("SELECT count(*) FROM `feedback` WHERE receiver=? AND reply_to_email=? AND feedback=? AND app_version=?")).
 		WithArgs("app@tum.de", "testing@example.com", "Hello without image", nil).
 		WillReturnRows(sqlmock.NewRows([]string{"count(*)"}))
-	s.mock.ExpectQuery(regexp.QuoteMeta("INSERT INTO `feedback` (`image_count`,`email_id`,`receiver`,`reply_to`,`feedback`,`latitude`,`longitude`,`os_version`,`app_version`,`processed`) VALUES (?,?,?,?,?,?,?,?,?,?) RETURNING `timestamp`,`id`")).
-		WithArgs(0, sqlmock.AnyArg(), "app@tum.de", "testing@example.com", "Hello without image", nil, nil, nil, nil, false).
+	s.mock.ExpectQuery(regexp.QuoteMeta("INSERT INTO `feedback` (`image_count`,`email_id`,`receiver`,`reply_to_email`,`reply_to_name`,`feedback`,`latitude`,`longitude`,`os_version`,`app_version`,`processed`) VALUES (?,?,?,?,?,?,?,?,?,?,?) RETURNING `timestamp`,`id`")).
+		WithArgs(0, sqlmock.AnyArg(), "app@tum.de", "testing@example.com", nil, "Hello without image", nil, nil, nil, nil, false).
 		WillReturnRows(sqlmock.NewRows([]string{"timestamp", "id"}).AddRow(time.Now(), 1))
 	s.mock.ExpectCommit()
 

server/backend/migration/20240405000000.go

@@ -0,0 +1,32 @@
+package migration
+
+import (
+	"github.com/go-gormigrate/gormigrate/v2"
+	"gorm.io/gorm"
+)
+
+// migrate20240405000000
+// Split the reply-to fields of the feedback model into name and email
+func migrate20240405000000() *gormigrate.Migration {
+	return &gormigrate.Migration{
+		ID: "20240405000000",
+		Migrate: func(tx *gorm.DB) error {
+			if err := tx.Exec("alter table feedback change reply_to reply_to_email text null").Error; err != nil {
+				return err
+			}
+			if err := tx.Exec("alter table feedback add reply_to_name text null default null after reply_to_email").Error; err != nil {
+				return err
+			}
+			return nil
+		},
+		Rollback: func(tx *gorm.DB) error {
+			if err := tx.Exec("alter table feedback change reply_to_email reply_to text null").Error; err != nil {
+				return err
+			}
+			if err := tx.Exec("alter table feedback drop column reply_to_name").Error; err != nil {
+				return err
+			}
+			return nil
+		},
+	}
+}

server/backend/migration/migration.go

@@ -79,6 +79,7 @@ func manualMigrate(db *gorm.DB) error {
 		migrate20240319000000(),
 		migrate20240327000000(),
 		migrate20240402000000(),
+		migrate20240405000000(),
 	}
 	return gormigrate.New(db, gormigrateOptions, migrations).Migrate()
 }

server/backend/rpcserver.go

@@ -1,15 +1,18 @@
 package backend
 
 import (
+	"sync"
+
 	pb "github.com/TUM-Dev/Campus-Backend/server/api/tumdev"
 	log "github.com/sirupsen/logrus"
 	"gorm.io/gorm"
 )
 
 type CampusServer struct {
 	pb.UnimplementedCampusServer
-	db        *gorm.DB
-	deviceBuf *deviceBuffer // deviceBuf stores all devices from recent request and flushes them to db
+	feedbackEmailLastReuestAt *sync.Map
+	db                        *gorm.DB
+	deviceBuf                 *deviceBuffer // deviceBuf stores all devices from recent request and flushes them to db
 }
 
 // Verify that CampusServer implements the pb.CampusServer interface
@@ -18,7 +21,8 @@ var _ pb.CampusServer = (*CampusServer)(nil)
 func New(db *gorm.DB) *CampusServer {
 	log.Trace("Server starting up")
 	return &CampusServer{
-		db:        db,
-		deviceBuf: newDeviceBuffer(),
+		db:                        db,
+		deviceBuf:                 newDeviceBuffer(),
+		feedbackEmailLastReuestAt: &sync.Map{},
 	}
 }

server/go.sum

@@ -464,12 +464,8 @@ golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8T
 golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
-google.golang.org/genproto/googleapis/api v0.0.0-20240325203815-454cdb8f5daa h1:Jt1XW5PaLXF1/ePZrznsh/aAUvI7Adfc3LY1dAKlzRs=
-google.golang.org/genproto/googleapis/api v0.0.0-20240325203815-454cdb8f5daa/go.mod h1:K4kfzHtI0kqWA79gecJarFtDn/Mls+GxQcg3Zox91Ac=
 google.golang.org/genproto/googleapis/api v0.0.0-20240401170217-c3f982113cda h1:b6F6WIV4xHHD0FA4oIyzU6mHWg2WI2X1RBehwa5QN38=
 google.golang.org/genproto/googleapis/api v0.0.0-20240401170217-c3f982113cda/go.mod h1:AHcE/gZH76Bk/ROZhQphlRoWo5xKDEtz3eVEO1LfA8c=
-google.golang.org/genproto/googleapis/rpc v0.0.0-20240318140521-94a12d6c2237 h1:NnYq6UN9ReLM9/Y01KWNOWyI5xQ9kbIms5GGJVwS/Yc=
-google.golang.org/genproto/googleapis/rpc v0.0.0-20240318140521-94a12d6c2237/go.mod h1:WtryC6hu0hhx87FDGxWCDptyssuo68sk10vYjF+T9fY=
 google.golang.org/genproto/googleapis/rpc v0.0.0-20240325203815-454cdb8f5daa h1:RBgMaUMP+6soRkik4VoN8ojR2nex2TqZwjSSogic+eo=
 google.golang.org/genproto/googleapis/rpc v0.0.0-20240325203815-454cdb8f5daa/go.mod h1:WtryC6hu0hhx87FDGxWCDptyssuo68sk10vYjF+T9fY=
 google.golang.org/grpc v1.62.1 h1:B4n+nfKzOICUXMgyrNd19h/I9oH0L1pizfk1d4zSgTk=

server/model/feedback.go

@@ -5,18 +5,19 @@ import (
 )
 
 type Feedback struct {
-	Id         int64       `gorm:"column:id;primary_key;AUTO_INCREMENT;type:int;not null;"`
-	ImageCount int32       `gorm:"column:image_count;type:int;not null;"`
-	EmailId    string      `gorm:"column:email_id;type:text;not null"`
-	Recipient  string      `gorm:"column:receiver;type:text;not null;uniqueIndex:receiver_reply_to_feedback_app_version_uindex"`
-	ReplyTo    null.String `gorm:"column:reply_to;type:text;null;uniqueIndex:receiver_reply_to_feedback_app_version_uindex"`
-	Feedback   string      `gorm:"column:feedback;type:text;not null;uniqueIndex:receiver_reply_to_feedback_app_version_uindex"`
-	Latitude   null.Float  `gorm:"column:latitude;type:float;null;"`
-	Longitude  null.Float  `gorm:"column:longitude;type:float;null;"`
-	OsVersion  null.String `gorm:"column:os_version;type:text;null;"`
-	AppVersion null.String `gorm:"column:app_version;type:text;null;uniqueIndex:receiver_reply_to_feedback_app_version_uindex"`
-	Processed  bool        `gorm:"column:processed;type:boolean;default:false;not null;"`
-	Timestamp  null.Time   `gorm:"column:timestamp;type:timestamp;default:CURRENT_TIMESTAMP;null;"`
+	Id           int64       `gorm:"column:id;primary_key;AUTO_INCREMENT;type:int;not null;"`
+	ImageCount   int32       `gorm:"column:image_count;type:int;not null;"`
+	EmailId      string      `gorm:"column:email_id;type:text;not null"`
+	Recipient    string      `gorm:"column:receiver;type:text;not null;uniqueIndex:receiver_reply_to_feedback_app_version_uindex"`
+	ReplyToEmail null.String `gorm:"column:reply_to_email;type:text;null;uniqueIndex:receiver_reply_to_feedback_app_version_uindex"`
+	ReplyToName  null.String `gorm:"column:reply_to_name;type:text;null"`
+	Feedback     string      `gorm:"column:feedback;type:text;not null;uniqueIndex:receiver_reply_to_feedback_app_version_uindex"`
+	Latitude     null.Float  `gorm:"column:latitude;type:float;null;"`
+	Longitude    null.Float  `gorm:"column:longitude;type:float;null;"`
+	OsVersion    null.String `gorm:"column:os_version;type:text;null;"`
+	AppVersion   null.String `gorm:"column:app_version;type:text;null;uniqueIndex:receiver_reply_to_feedback_app_version_uindex"`
+	Processed    bool        `gorm:"column:processed;type:boolean;default:false;not null;"`
+	Timestamp    null.Time   `gorm:"column:timestamp;type:timestamp;default:CURRENT_TIMESTAMP;null;"`
 }
 
 // TableName sets the insert table name for this struct type