Add bandwidth limit to tier; fix display name sync issues

This commit is contained in:
binwiederhier 2023-01-25 10:05:54 -05:00
parent 1771cb3fdb
commit 236254d907
13 changed files with 119 additions and 51 deletions

View file

@ -101,7 +101,7 @@ type Config struct {
TotalAttachmentSizeLimit int64
VisitorSubscriptionLimit int
VisitorAttachmentTotalSizeLimit int64
VisitorAttachmentDailyBandwidthLimit int
VisitorAttachmentDailyBandwidthLimit int64
VisitorRequestLimitBurst int
VisitorRequestLimitReplenish time.Duration
VisitorRequestExemptIPAddrs []netip.Prefix

View file

@ -40,7 +40,6 @@ TODO
- HIGH Rate limiting: dailyLimitToRate is wrong? + TESTS
- HIGH Rate limiting: Sensitive endpoints (account/login/change-password/...)
- HIGH Rate limiting: Bandwidth limit must be in tier + TESTS
- MEDIUM: Races with v.user (see publishSyncEventAsync test)
- MEDIUM: Reservation (UI): Show "This topic is reserved" error message when trying to reserve a reserved topic (Thorben)
- MEDIUM: Reservation (UI): Ask for confirmation when removing reservation (deadcade)
@ -866,7 +865,6 @@ func (s *Server) handleBodyAsAttachment(r *http.Request, v *visitor, m *message,
util.NewFixedLimiter(vinfo.Limits.AttachmentFileSizeLimit),
util.NewFixedLimiter(vinfo.Stats.AttachmentTotalSizeRemaining),
}
fmt.Printf("limiters = %#v\nv = %#v\n", limiters, v)
m.Attachment.Size, err = s.fileCache.Write(m.ID, body, limiters...)
if err == util.ErrLimitReached {
return errHTTPEntityTooLargeAttachment

View file

@ -11,6 +11,7 @@ import (
const (
subscriptionIDLength = 16
subscriptionIDPrefix = "su_"
syncTopicAccountSyncEvent = "sync"
)
@ -55,6 +56,7 @@ func (s *Server) handleAccountGet(w http.ResponseWriter, _ *http.Request, v *vis
AttachmentTotalSize: limits.AttachmentTotalSizeLimit,
AttachmentFileSize: limits.AttachmentFileSizeLimit,
AttachmentExpiryDuration: int64(limits.AttachmentExpiryDuration.Seconds()),
AttachmentBandwidth: limits.AttachmentBandwidthLimit,
},
Stats: &apiAccountStats{
Messages: stats.Messages,
@ -249,7 +251,7 @@ func (s *Server) handleAccountSubscriptionAdd(w http.ResponseWriter, r *http.Req
}
}
if newSubscription.ID == "" {
newSubscription.ID = util.RandomString(subscriptionIDLength)
newSubscription.ID = util.RandomStringPrefix(subscriptionIDPrefix, subscriptionIDLength)
v.user.Prefs.Subscriptions = append(v.user.Prefs.Subscriptions, newSubscription)
if err := s.userManager.ChangeSettings(v.user); err != nil {
return err

View file

@ -153,9 +153,9 @@ func TestAccount_ChangeSettings(t *testing.T) {
require.Equal(t, 200, rr.Code)
account, _ := util.UnmarshalJSON[apiAccountResponse](io.NopCloser(rr.Body))
require.Equal(t, "de", account.Language)
require.Equal(t, 86400, account.Notification.DeleteAfter)
require.Equal(t, "juntos", account.Notification.Sound)
require.Equal(t, 0, account.Notification.MinPriority) // Not set
require.Equal(t, util.Int(86400), account.Notification.DeleteAfter)
require.Equal(t, util.String("juntos"), account.Notification.Sound)
require.Nil(t, account.Notification.MinPriority) // Not set
}
func TestAccount_Subscription_AddUpdateDelete(t *testing.T) {
@ -176,7 +176,7 @@ func TestAccount_Subscription_AddUpdateDelete(t *testing.T) {
require.NotEmpty(t, account.Subscriptions[0].ID)
require.Equal(t, "http://abc.com", account.Subscriptions[0].BaseURL)
require.Equal(t, "def", account.Subscriptions[0].Topic)
require.Equal(t, "", account.Subscriptions[0].DisplayName)
require.Nil(t, account.Subscriptions[0].DisplayName)
subscriptionID := account.Subscriptions[0].ID
rr = request(t, s, "PATCH", "/v1/account/subscription/"+subscriptionID, `{"display_name": "ding dong"}`, map[string]string{
@ -193,7 +193,7 @@ func TestAccount_Subscription_AddUpdateDelete(t *testing.T) {
require.Equal(t, subscriptionID, account.Subscriptions[0].ID)
require.Equal(t, "http://abc.com", account.Subscriptions[0].BaseURL)
require.Equal(t, "def", account.Subscriptions[0].Topic)
require.Equal(t, "ding dong", account.Subscriptions[0].DisplayName)
require.Equal(t, util.String("ding dong"), account.Subscriptions[0].DisplayName)
rr = request(t, s, "DELETE", "/v1/account/subscription/"+subscriptionID, "", map[string]string{
"Authorization": util.BasicAuth("phil", "phil"),
@ -402,6 +402,7 @@ func TestAccount_Reservation_AddRemoveUserWithTierSuccess(t *testing.T) {
AttachmentFileSizeLimit: 1231231,
AttachmentTotalSizeLimit: 123123,
AttachmentExpiryDuration: 10800 * time.Second,
AttachmentBandwidthLimit: 21474836480,
}))
require.Nil(t, s.userManager.ChangeTier("phil", "pro"))
@ -442,6 +443,7 @@ func TestAccount_Reservation_AddRemoveUserWithTierSuccess(t *testing.T) {
require.Equal(t, int64(1231231), account.Limits.AttachmentFileSize)
require.Equal(t, int64(123123), account.Limits.AttachmentTotalSize)
require.Equal(t, int64(10800), account.Limits.AttachmentExpiryDuration)
require.Equal(t, int64(21474836480), account.Limits.AttachmentBandwidth)
require.Equal(t, 2, len(account.Reservations))
require.Equal(t, "another", account.Reservations[0].Topic)
require.Equal(t, "write-only", account.Reservations[0].Everyone)

View file

@ -265,6 +265,7 @@ func TestPayments_Webhook_Subscription_Updated_Downgrade_From_PastDue_To_Active(
AttachmentExpiryDuration: time.Hour,
AttachmentFileSizeLimit: 1000000,
AttachmentTotalSizeLimit: 1000000,
AttachmentBandwidthLimit: 1000000,
}))
require.Nil(t, s.userManager.CreateTier(&user.Tier{
Code: "pro",
@ -275,6 +276,7 @@ func TestPayments_Webhook_Subscription_Updated_Downgrade_From_PastDue_To_Active(
AttachmentExpiryDuration: time.Hour,
AttachmentFileSizeLimit: 1000000,
AttachmentTotalSizeLimit: 1000000,
AttachmentBandwidthLimit: 1000000,
}))
require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser))
require.Nil(t, s.userManager.ChangeTier("phil", "pro"))

View file

@ -1368,6 +1368,7 @@ func TestServer_PublishAttachmentWithTierBasedExpiry(t *testing.T) {
AttachmentFileSizeLimit: 50_000,
AttachmentTotalSizeLimit: 200_000,
AttachmentExpiryDuration: sevenDays, // 7 days
AttachmentBandwidthLimit: 100000,
}))
require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser))
require.Nil(t, s.userManager.ChangeTier("phil", "test"))
@ -1376,6 +1377,7 @@ func TestServer_PublishAttachmentWithTierBasedExpiry(t *testing.T) {
response := request(t, s, "PUT", "/mytopic", content, map[string]string{
"Authorization": util.BasicAuth("phil", "phil"),
})
require.Equal(t, 200, response.Code)
msg := toMessage(t, response.Body.String())
require.Contains(t, msg.Attachment.URL, "http://127.0.0.1:12345/file/")
require.True(t, msg.Attachment.Expires > time.Now().Add(sevenDays-30*time.Second).Unix())
@ -1396,6 +1398,46 @@ func TestServer_PublishAttachmentWithTierBasedExpiry(t *testing.T) {
require.Equal(t, 200, response.Code)
}
func TestServer_PublishAttachmentWithTierBasedBandwidthLimit(t *testing.T) {
content := util.RandomString(5000) // > 4096
c := newTestConfigWithAuthFile(t)
s := newTestServer(t, c)
// Create tier with certain limits
require.Nil(t, s.userManager.CreateTier(&user.Tier{
Code: "test",
MessagesLimit: 10,
MessagesExpiryDuration: time.Hour,
AttachmentFileSizeLimit: 50_000,
AttachmentTotalSizeLimit: 200_000,
AttachmentExpiryDuration: time.Hour,
AttachmentBandwidthLimit: 14000, // < 3x5000 bytes -> enough for one upload, one download
}))
require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser))
require.Nil(t, s.userManager.ChangeTier("phil", "test"))
// Publish and make sure we can retrieve it
rr := request(t, s, "PUT", "/mytopic", content, map[string]string{
"Authorization": util.BasicAuth("phil", "phil"),
})
require.Equal(t, 200, rr.Code)
msg := toMessage(t, rr.Body.String())
// Retrieve it (first time succeeds)
rr = request(t, s, "GET", "/file/"+msg.ID, content, map[string]string{
"Authorization": util.BasicAuth("phil", "phil"),
})
require.Equal(t, 200, rr.Code)
require.Equal(t, content, rr.Body.String())
// Retrieve it AGAIN (fails, due to bandwidth limit)
rr = request(t, s, "GET", "/file/"+msg.ID, content, map[string]string{
"Authorization": util.BasicAuth("phil", "phil"),
})
require.Equal(t, 429, rr.Code)
}
func TestServer_PublishAttachmentWithTierBasedLimits(t *testing.T) {
smallFile := util.RandomString(20_000)
largeFile := util.RandomString(50_000)
@ -1412,6 +1454,7 @@ func TestServer_PublishAttachmentWithTierBasedLimits(t *testing.T) {
AttachmentFileSizeLimit: 50_000,
AttachmentTotalSizeLimit: 200_000,
AttachmentExpiryDuration: 30 * time.Second,
AttachmentBandwidthLimit: 1000000,
}))
require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser))
require.Nil(t, s.userManager.ChangeTier("phil", "test"))

View file

@ -246,7 +246,7 @@ type apiAccountTier struct {
}
type apiAccountLimits struct {
Basis string `json:"basis,omitempty"` // "ip", "role" or "tier"
Basis string `json:"basis,omitempty"` // "ip" or "tier"
Messages int64 `json:"messages"`
MessagesExpiryDuration int64 `json:"messages_expiry_duration"`
Emails int64 `json:"emails"`
@ -254,6 +254,7 @@ type apiAccountLimits struct {
AttachmentTotalSize int64 `json:"attachment_total_size"`
AttachmentFileSize int64 `json:"attachment_file_size"`
AttachmentExpiryDuration int64 `json:"attachment_expiry_duration"`
AttachmentBandwidth int64 `json:"attachment_bandwidth"`
}
type apiAccountStats struct {

View file

@ -31,9 +31,9 @@ var (
type visitor struct {
config *Config
messageCache *messageCache
userManager *user.Manager // May be nil!
ip netip.Addr
user *user.User
userManager *user.Manager // May be nil
ip netip.Addr // Visitor IP address
user *user.User // Only set if authenticated user, otherwise nil
messages int64 // Number of messages sent, reset every day
emails int64 // Number of emails sent, reset every day
requestLimiter *rate.Limiter // Rate limiter for (almost) all requests (including messages)
@ -61,6 +61,7 @@ type visitorLimits struct {
AttachmentTotalSizeLimit int64
AttachmentFileSizeLimit int64
AttachmentExpiryDuration time.Duration
AttachmentBandwidthLimit int64
}
type visitorStats struct {
@ -84,7 +85,7 @@ const (
)
func newVisitor(conf *Config, messageCache *messageCache, userManager *user.Manager, ip netip.Addr, user *user.User) *visitor {
var messagesLimiter util.Limiter
var messagesLimiter, attachmentBandwidthLimiter util.Limiter
var requestLimiter, emailsLimiter, accountLimiter *rate.Limiter
var messages, emails int64
if user != nil {
@ -97,9 +98,11 @@ func newVisitor(conf *Config, messageCache *messageCache, userManager *user.Mana
requestLimiter = rate.NewLimiter(dailyLimitToRate(user.Tier.MessagesLimit), conf.VisitorRequestLimitBurst)
messagesLimiter = util.NewFixedLimiter(user.Tier.MessagesLimit)
emailsLimiter = rate.NewLimiter(dailyLimitToRate(user.Tier.EmailsLimit), conf.VisitorEmailLimitBurst)
attachmentBandwidthLimiter = util.NewBytesLimiter(int(user.Tier.AttachmentBandwidthLimit), 24*time.Hour)
} else {
requestLimiter = rate.NewLimiter(rate.Every(conf.VisitorRequestLimitReplenish), conf.VisitorRequestLimitBurst)
emailsLimiter = rate.NewLimiter(rate.Every(conf.VisitorEmailLimitReplenish), conf.VisitorEmailLimitBurst)
attachmentBandwidthLimiter = util.NewBytesLimiter(int(conf.VisitorAttachmentDailyBandwidthLimit), 24*time.Hour)
}
return &visitor{
config: conf,
@ -113,7 +116,7 @@ func newVisitor(conf *Config, messageCache *messageCache, userManager *user.Mana
messagesLimiter: messagesLimiter, // May be nil
emailsLimiter: emailsLimiter,
subscriptionLimiter: util.NewFixedLimiter(int64(conf.VisitorSubscriptionLimit)),
bandwidthLimiter: util.NewBytesLimiter(conf.VisitorAttachmentDailyBandwidthLimit, 24*time.Hour),
bandwidthLimiter: attachmentBandwidthLimiter,
accountLimiter: accountLimiter, // May be nil
firebase: time.Unix(0, 0),
seen: time.Now(),
@ -259,6 +262,7 @@ func (v *visitor) Limits() *visitorLimits {
limits.AttachmentTotalSizeLimit = v.user.Tier.AttachmentTotalSizeLimit
limits.AttachmentFileSizeLimit = v.user.Tier.AttachmentFileSizeLimit
limits.AttachmentExpiryDuration = v.user.Tier.AttachmentExpiryDuration
limits.AttachmentBandwidthLimit = v.user.Tier.AttachmentBandwidthLimit
}
return limits
}
@ -327,5 +331,6 @@ func defaultVisitorLimits(conf *Config) *visitorLimits {
AttachmentTotalSizeLimit: conf.VisitorAttachmentTotalSizeLimit,
AttachmentFileSizeLimit: conf.AttachmentFileSizeLimit,
AttachmentExpiryDuration: conf.AttachmentExpiryDuration,
AttachmentBandwidthLimit: conf.VisitorAttachmentDailyBandwidthLimit,
}
}