Tiers make sense for admins now

2023-01-09 15:40:46 -05:00 · 2023-01-09 15:40:46 -05:00 · 3aba7404fc
commit 3aba7404fc
parent d8032e1c9e
18 changed files with 457 additions and 225 deletions
--- a/server/errors.go
+++ b/server/errors.go
@ -73,6 +73,7 @@ var (
 	errHTTPTooManyRequestsLimitAttachmentBandwidth   = &errHTTP{42905, http.StatusTooManyRequests, "limit reached: daily bandwidth", "https://ntfy.sh/docs/publish/#limitations"}
 	errHTTPTooManyRequestsLimitAccountCreation       = &errHTTP{42906, http.StatusTooManyRequests, "limit reached: too many accounts created", "https://ntfy.sh/docs/publish/#limitations"} // FIXME document limit
 	errHTTPTooManyRequestsLimitReservations          = &errHTTP{42907, http.StatusTooManyRequests, "limit reached: too many topic reservations for this user", ""}
+	errHTTPTooManyRequestsLimitMessages              = &errHTTP{42908, http.StatusTooManyRequests, "limit reached: too many messages", "https://ntfy.sh/docs/publish/#limitations"}
 	errHTTPInternalError                             = &errHTTP{50001, http.StatusInternalServerError, "internal server error", ""}
 	errHTTPInternalErrorInvalidPath                  = &errHTTP{50002, http.StatusInternalServerError, "internal server error: invalid path", ""}
 	errHTTPInternalErrorMissingBaseURL               = &errHTTP{50003, http.StatusInternalServerError, "internal server error: base-url must be be configured for this feature", "https://ntfy.sh/docs/config/"}
--- a/server/server.go
+++ b/server/server.go
@ -36,16 +36,14 @@ import (

 /*
 	TODO
-		limits & rate limiting:
+		Limits & rate limiting:
 			login/account endpoints
-		plan:
-			weirdness with admin and "default" account
-		v.Info() endpoint double selects from DB
 		purge accounts that were not logged int o in X
-		reset daily limits for users
+		reset daily Limits for users
 		Make sure account endpoints make sense for admins
 		add logic to set "expires" column (this is gonna be dirty)
 		UI:
+		- Align size of message bar and upgrade banner
 		- flicker of upgrade banner
 		- JS constants
 		- useContext for account
@ -53,8 +51,10 @@ import (
 			- "account topic" sync mechanism
 			- "mute" setting
 			- figure out what settings are "web" or "phone"
+		Delete visitor when tier is changed to refresh rate limiters
 		Tests:
-		- visitor with/without user
+		- Change tier from higher to lower tier (delete reservations)
+		- Message rate limiting and reset tests
 		Docs:
 		- "expires" field in message
 		Refactor:
@ -528,7 +528,7 @@ func (s *Server) handlePublishWithoutResponse(r *http.Request, v *visitor) (*mes
 		return nil, err
 	}
 	if err := v.MessageAllowed(); err != nil {
-		return nil, errHTTPTooManyRequestsLimitRequests // FIXME make one for messages
+		return nil, errHTTPTooManyRequestsLimitMessages
 	}
 	body, err := util.Peek(r.Body, s.config.MessageLimit)
 	if err != nil {
@ -545,11 +545,7 @@ func (s *Server) handlePublishWithoutResponse(r *http.Request, v *visitor) (*mes
 	if v.user != nil {
 		m.User = v.user.Name
 	}
-	if v.user != nil && v.user.Tier != nil {
-		m.Expires = time.Now().Unix() + v.user.Tier.MessagesExpiryDuration
-	} else {
-		m.Expires = time.Now().Add(s.config.CacheDuration).Unix()
-	}
+	m.Expires = time.Now().Add(v.Limits().MessagesExpiryDuration).Unix()
 	if err := s.handlePublishBody(r, v, m, body, unifiedpush); err != nil {
 		return nil, err
 	}
@ -822,24 +818,18 @@ func (s *Server) handleBodyAsAttachment(r *http.Request, v *visitor, m *message,
 	if s.fileCache == nil || s.config.BaseURL == "" || s.config.AttachmentCacheDir == "" {
 		return errHTTPBadRequestAttachmentsDisallowed
 	}
-	var attachmentExpiryDuration time.Duration
-	if v.user != nil && v.user.Tier != nil {
-		attachmentExpiryDuration = time.Duration(v.user.Tier.AttachmentExpiryDuration) * time.Second
-	} else {
-		attachmentExpiryDuration = s.config.AttachmentExpiryDuration
-	}
-	attachmentExpiry := time.Now().Add(attachmentExpiryDuration).Unix()
-	if m.Time > attachmentExpiry {
-		return errHTTPBadRequestAttachmentsExpiryBeforeDelivery
-	}
-	stats, err := v.Info()
+	vinfo, err := v.Info()
 	if err != nil {
 		return err
 	}
+	attachmentExpiry := time.Now().Add(vinfo.Limits.AttachmentExpiryDuration).Unix()
+	if m.Time > attachmentExpiry {
+		return errHTTPBadRequestAttachmentsExpiryBeforeDelivery
+	}
 	contentLengthStr := r.Header.Get("Content-Length")
 	if contentLengthStr != "" { // Early "do-not-trust" check, hard limit see below
 		contentLength, err := strconv.ParseInt(contentLengthStr, 10, 64)
-		if err == nil && (contentLength > stats.AttachmentTotalSizeRemaining || contentLength > stats.AttachmentFileSizeLimit) {
+		if err == nil && (contentLength > vinfo.Stats.AttachmentTotalSizeRemaining || contentLength > vinfo.Limits.AttachmentFileSizeLimit) {
 			return errHTTPEntityTooLargeAttachment
 		}
 	}
@ -859,8 +849,8 @@ func (s *Server) handleBodyAsAttachment(r *http.Request, v *visitor, m *message,
 	}
 	limiters := []util.Limiter{
 		v.BandwidthLimiter(),
-		util.NewFixedLimiter(stats.AttachmentFileSizeLimit),
-		util.NewFixedLimiter(stats.AttachmentTotalSizeRemaining),
+		util.NewFixedLimiter(vinfo.Limits.AttachmentFileSizeLimit),
+		util.NewFixedLimiter(vinfo.Stats.AttachmentTotalSizeRemaining),
 	}
 	m.Attachment.Size, err = s.fileCache.Write(m.ID, body, limiters...)
 	if err == util.ErrLimitReached {
--- a/server/server_account.go
+++ b/server/server_account.go
@ -40,11 +40,22 @@ func (s *Server) handleAccountCreate(w http.ResponseWriter, r *http.Request, v *
 }

 func (s *Server) handleAccountGet(w http.ResponseWriter, _ *http.Request, v *visitor) error {
-	stats, err := v.Info()
+	info, err := v.Info()
 	if err != nil {
 		return err
 	}
+	limits, stats := info.Limits, info.Stats
 	response := &apiAccountResponse{
+		Limits: &apiAccountLimits{
+			Basis:                    string(limits.Basis),
+			Messages:                 limits.MessagesLimit,
+			MessagesExpiryDuration:   int64(limits.MessagesExpiryDuration.Seconds()),
+			Emails:                   limits.EmailsLimit,
+			Reservations:             limits.ReservationsLimit,
+			AttachmentTotalSize:      limits.AttachmentTotalSizeLimit,
+			AttachmentFileSize:       limits.AttachmentFileSizeLimit,
+			AttachmentExpiryDuration: int64(limits.AttachmentExpiryDuration.Seconds()),
+		},
 		Stats: &apiAccountStats{
 			Messages:                     stats.Messages,
 			MessagesRemaining:            stats.MessagesRemaining,
@ -55,16 +66,6 @@ func (s *Server) handleAccountGet(w http.ResponseWriter, _ *http.Request, v *vis
 			AttachmentTotalSize:          stats.AttachmentTotalSize,
 			AttachmentTotalSizeRemaining: stats.AttachmentTotalSizeRemaining,
 		},
-		Limits: &apiAccountLimits{
-			Basis:                    stats.Basis,
-			Messages:                 stats.MessagesLimit,
-			MessagesExpiryDuration:   stats.MessagesExpiryDuration,
-			Emails:                   stats.EmailsLimit,
-			Reservations:             stats.ReservationsLimit,
-			AttachmentTotalSize:      stats.AttachmentTotalSizeLimit,
-			AttachmentFileSize:       stats.AttachmentFileSizeLimit,
-			AttachmentExpiryDuration: stats.AttachmentExpiryDuration,
-		},
 	}
 	if v.user != nil {
 		response.Username = v.user.Name
@ -82,18 +83,9 @@ func (s *Server) handleAccountGet(w http.ResponseWriter, _ *http.Request, v *vis
 		}
 		if v.user.Tier != nil {
 			response.Tier = &apiAccountTier{
-				Code:        v.user.Tier.Code,
-				Upgradeable: v.user.Tier.Upgradeable,
-			}
-		} else if v.user.Role == user.RoleAdmin {
-			response.Tier = &apiAccountTier{
-				Code:        string(user.TierUnlimited),
-				Upgradeable: false,
-			}
-		} else {
-			response.Tier = &apiAccountTier{
-				Code:        string(user.TierDefault),
-				Upgradeable: true,
+				Code: v.user.Tier.Code,
+				Name: v.user.Tier.Name,
+				Paid: v.user.Tier.Paid,
 			}
 		}
 		reservations, err := s.userManager.Reservations(v.user.Name)
@ -112,10 +104,6 @@ func (s *Server) handleAccountGet(w http.ResponseWriter, _ *http.Request, v *vis
 	} else {
 		response.Username = user.Everyone
 		response.Role = string(user.RoleAnonymous)
-		response.Tier = &apiAccountTier{
-			Code:        string(user.TierNone),
-			Upgradeable: true,
-		}
 	}
 	w.Header().Set("Content-Type", "application/json")
 	w.Header().Set("Access-Control-Allow-Origin", "*") // FIXME remove this
--- a/server/server_account_test.go
+++ b/server/server_account_test.go
@ -381,14 +381,14 @@ func TestAccount_Reservation_AddRemoveUserWithTierSuccess(t *testing.T) {
 	// Create a tier
 	require.Nil(t, s.userManager.CreateTier(&user.Tier{
 		Code:                     "pro",
-		Upgradeable:              false,
+		Paid:                     false,
 		MessagesLimit:            123,
-		MessagesExpiryDuration:   86400,
+		MessagesExpiryDuration:   86400 * time.Second,
 		EmailsLimit:              32,
 		ReservationsLimit:        2,
 		AttachmentFileSizeLimit:  1231231,
 		AttachmentTotalSizeLimit: 123123,
-		AttachmentExpiryDuration: 10800,
+		AttachmentExpiryDuration: 10800 * time.Second,
 	}))
 	require.Nil(t, s.userManager.ChangeTier("phil", "pro"))

--- a/server/server_test.go
+++ b/server/server_test.go
@ -1098,7 +1098,7 @@ func TestServer_PublishWithTierBasedMessageLimitAndExpiry(t *testing.T) {
 	require.Nil(t, s.userManager.CreateTier(&user.Tier{
 		Code:                   "test",
 		MessagesLimit:          5,
-		MessagesExpiryDuration: -5, // Second, what a hack!
+		MessagesExpiryDuration: -5 * time.Second, // Second, what a hack!
 	}))
 	require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser))
 	require.Nil(t, s.userManager.ChangeTier("phil", "test"))
@ -1323,14 +1323,14 @@ func TestServer_PublishAttachmentWithTierBasedExpiry(t *testing.T) {
 	s := newTestServer(t, c)

 	// Create tier with certain limits
-	sevenDaysInSeconds := int64(604800)
+	sevenDays := time.Duration(604800) * time.Second
 	require.Nil(t, s.userManager.CreateTier(&user.Tier{
 		Code:                     "test",
 		MessagesLimit:            10,
-		MessagesExpiryDuration:   sevenDaysInSeconds,
+		MessagesExpiryDuration:   sevenDays,
 		AttachmentFileSizeLimit:  50_000,
 		AttachmentTotalSizeLimit: 200_000,
-		AttachmentExpiryDuration: sevenDaysInSeconds, // 7 days
+		AttachmentExpiryDuration: sevenDays, // 7 days
 	}))
 	require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser))
 	require.Nil(t, s.userManager.ChangeTier("phil", "test"))
@ -1341,8 +1341,8 @@ func TestServer_PublishAttachmentWithTierBasedExpiry(t *testing.T) {
 	})
 	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().Unix()+sevenDaysInSeconds-30)
-	require.True(t, msg.Expires > time.Now().Unix()+sevenDaysInSeconds-30)
+	require.True(t, msg.Attachment.Expires > time.Now().Add(sevenDays-30*time.Second).Unix())
+	require.True(t, msg.Expires > time.Now().Add(sevenDays-30*time.Second).Unix())
 	file := filepath.Join(s.config.AttachmentCacheDir, msg.ID)
 	require.FileExists(t, file)

@ -1374,7 +1374,7 @@ func TestServer_PublishAttachmentWithTierBasedLimits(t *testing.T) {
 		MessagesLimit:            100,
 		AttachmentFileSizeLimit:  50_000,
 		AttachmentTotalSizeLimit: 200_000,
-		AttachmentExpiryDuration: 30,
+		AttachmentExpiryDuration: 30 * time.Second,
 	}))
 	require.Nil(t, s.userManager.AddUser("phil", "phil", user.RoleUser))
 	require.Nil(t, s.userManager.ChangeTier("phil", "test"))
--- a/server/types.go
+++ b/server/types.go
@ -236,8 +236,9 @@ type apiAccountTokenResponse struct {
 }

 type apiAccountTier struct {
-	Code        string `json:"code"`
-	Upgradeable bool   `json:"upgradeable"`
+	Code string `json:"code"`
+	Name string `json:"name"`
+	Paid bool   `json:"paid"`
 }

 type apiAccountLimits struct {
--- a/server/visitor.go
+++ b/server/visitor.go
@ -38,29 +38,46 @@ type visitor struct {
 	bandwidthLimiter    util.Limiter  // Limiter for attachment bandwidth downloads
 	accountLimiter      *rate.Limiter // Rate limiter for account creation
 	firebase            time.Time     // Next allowed Firebase message
-	seen                time.Time
+	seen                time.Time     // Last seen time of this visitor (needed for removal of stale visitors)
 	mu                  sync.Mutex
 }

 type visitorInfo struct {
-	Basis                        string // "ip", "role" or "tier"
+	Limits *visitorLimits
+	Stats  *visitorStats
+}
+
+type visitorLimits struct {
+	Basis                    visitorLimitBasis
+	MessagesLimit            int64
+	MessagesExpiryDuration   time.Duration
+	EmailsLimit              int64
+	ReservationsLimit        int64
+	AttachmentTotalSizeLimit int64
+	AttachmentFileSizeLimit  int64
+	AttachmentExpiryDuration time.Duration
+}
+
+type visitorStats struct {
 	Messages                     int64
-	MessagesLimit                int64
 	MessagesRemaining            int64
-	MessagesExpiryDuration       int64
 	Emails                       int64
-	EmailsLimit                  int64
 	EmailsRemaining              int64
 	Reservations                 int64
-	ReservationsLimit            int64
 	ReservationsRemaining        int64
 	AttachmentTotalSize          int64
-	AttachmentTotalSizeLimit     int64
 	AttachmentTotalSizeRemaining int64
-	AttachmentFileSizeLimit      int64
-	AttachmentExpiryDuration     int64
 }

+// visitorLimitBasis describes how the visitor limits were derived, either from a user's
+// IP address (default config), or from its tier
+type visitorLimitBasis string
+
+const (
+	visitorLimitBasisIP   = visitorLimitBasis("ip")
+	visitorLimitBasisTier = visitorLimitBasis("tier")
+)
+
 func newVisitor(conf *Config, messageCache *messageCache, userManager *user.Manager, ip netip.Addr, user *user.User) *visitor {
 	var messagesLimiter util.Limiter
 	var requestLimiter, emailsLimiter, accountLimiter *rate.Limiter
@ -82,13 +99,13 @@ func newVisitor(conf *Config, messageCache *messageCache, userManager *user.Mana
 	return &visitor{
 		config:              conf,
 		messageCache:        messageCache,
-		userManager:         userManager, // May be nil!
+		userManager:         userManager, // May be nil
 		ip:                  ip,
 		user:                user,
 		messages:            messages,
 		emails:              emails,
 		requestLimiter:      requestLimiter,
-		messagesLimiter:     messagesLimiter,
+		messagesLimiter:     messagesLimiter, // May be nil
 		emailsLimiter:       emailsLimiter,
 		subscriptionLimiter: util.NewFixedLimiter(int64(conf.VisitorSubscriptionLimit)),
 		bandwidthLimiter:    util.NewBytesLimiter(conf.VisitorAttachmentDailyBandwidthLimit, 24*time.Hour),
@ -183,37 +200,36 @@ func (v *visitor) IncrEmails() {
 	}
 }

+func (v *visitor) Limits() *visitorLimits {
+	limits := &visitorLimits{}
+	if v.user != nil && v.user.Tier != nil {
+		limits.Basis = visitorLimitBasisTier
+		limits.MessagesLimit = v.user.Tier.MessagesLimit
+		limits.MessagesExpiryDuration = v.user.Tier.MessagesExpiryDuration
+		limits.EmailsLimit = v.user.Tier.EmailsLimit
+		limits.ReservationsLimit = v.user.Tier.ReservationsLimit
+		limits.AttachmentTotalSizeLimit = v.user.Tier.AttachmentTotalSizeLimit
+		limits.AttachmentFileSizeLimit = v.user.Tier.AttachmentFileSizeLimit
+		limits.AttachmentExpiryDuration = v.user.Tier.AttachmentExpiryDuration
+	} else {
+		limits.Basis = visitorLimitBasisIP
+		limits.MessagesLimit = replenishDurationToDailyLimit(v.config.VisitorRequestLimitReplenish)
+		limits.MessagesExpiryDuration = v.config.CacheDuration
+		limits.EmailsLimit = replenishDurationToDailyLimit(v.config.VisitorEmailLimitReplenish)
+		limits.ReservationsLimit = 0 // No reservations for anonymous users, or users without a tier
+		limits.AttachmentTotalSizeLimit = v.config.VisitorAttachmentTotalSizeLimit
+		limits.AttachmentFileSizeLimit = v.config.AttachmentFileSizeLimit
+		limits.AttachmentExpiryDuration = v.config.AttachmentExpiryDuration
+	}
+	return limits
+}
+
 func (v *visitor) Info() (*visitorInfo, error) {
 	v.mu.Lock()
 	messages := v.messages
 	emails := v.emails
 	v.mu.Unlock()
-	info := &visitorInfo{}
-	if v.user != nil && v.user.Role == user.RoleAdmin {
-		info.Basis = "role"
-		// All limits are zero!
-		info.MessagesExpiryDuration = 24 * 3600   // FIXME this is awful. Should be from the Unlimited plan
-		info.AttachmentExpiryDuration = 24 * 3600 // FIXME this is awful. Should be from the Unlimited plan
-	} else if v.user != nil && v.user.Tier != nil {
-		info.Basis = "tier"
-		info.MessagesLimit = v.user.Tier.MessagesLimit
-		info.MessagesExpiryDuration = v.user.Tier.MessagesExpiryDuration
-		info.EmailsLimit = v.user.Tier.EmailsLimit
-		info.ReservationsLimit = v.user.Tier.ReservationsLimit
-		info.AttachmentTotalSizeLimit = v.user.Tier.AttachmentTotalSizeLimit
-		info.AttachmentFileSizeLimit = v.user.Tier.AttachmentFileSizeLimit
-		info.AttachmentExpiryDuration = v.user.Tier.AttachmentExpiryDuration
-	} else {
-		info.Basis = "ip"
-		info.MessagesLimit = replenishDurationToDailyLimit(v.config.VisitorRequestLimitReplenish)
-		info.MessagesExpiryDuration = int64(v.config.CacheDuration.Seconds())
-		info.EmailsLimit = replenishDurationToDailyLimit(v.config.VisitorEmailLimitReplenish)
-		info.ReservationsLimit = 0 // FIXME
-		info.AttachmentTotalSizeLimit = v.config.VisitorAttachmentTotalSizeLimit
-		info.AttachmentFileSizeLimit = v.config.AttachmentFileSizeLimit
-		info.AttachmentExpiryDuration = int64(v.config.AttachmentExpiryDuration.Seconds())
-	}
-	var attachmentsBytesUsed int64 // FIXME Maybe move this to endpoint?
+	var attachmentsBytesUsed int64
 	var err error
 	if v.user != nil {
 		attachmentsBytesUsed, err = v.messageCache.AttachmentBytesUsedByUser(v.user.Name)
@ -225,20 +241,26 @@ func (v *visitor) Info() (*visitorInfo, error) {
 	}
 	var reservations int64
 	if v.user != nil && v.userManager != nil {
-		reservations, err = v.userManager.ReservationsCount(v.user.Name) // FIXME dup call, move this to endpoint?
+		reservations, err = v.userManager.ReservationsCount(v.user.Name)
 		if err != nil {
 			return nil, err
 		}
 	}
-	info.Messages = messages
-	info.MessagesRemaining = zeroIfNegative(info.MessagesLimit - info.Messages)
-	info.Emails = emails
-	info.EmailsRemaining = zeroIfNegative(info.EmailsLimit - info.Emails)
-	info.Reservations = reservations
-	info.ReservationsRemaining = zeroIfNegative(info.ReservationsLimit - info.Reservations)
-	info.AttachmentTotalSize = attachmentsBytesUsed
-	info.AttachmentTotalSizeRemaining = zeroIfNegative(info.AttachmentTotalSizeLimit - info.AttachmentTotalSize)
-	return info, nil
+	limits := v.Limits()
+	stats := &visitorStats{
+		Messages:                     messages,
+		MessagesRemaining:            zeroIfNegative(limits.MessagesLimit - messages),
+		Emails:                       emails,
+		EmailsRemaining:              zeroIfNegative(limits.EmailsLimit - emails),
+		Reservations:                 reservations,
+		ReservationsRemaining:        zeroIfNegative(limits.ReservationsLimit - reservations),
+		AttachmentTotalSize:          attachmentsBytesUsed,
+		AttachmentTotalSizeRemaining: zeroIfNegative(limits.AttachmentTotalSizeLimit - attachmentsBytesUsed),
+	}
+	return &visitorInfo{
+		Limits: limits,
+		Stats:  stats,
+	}, nil
 }

 func zeroIfNegative(value int64) int64 {