From 1c4420bca8ec67915a9c120bc36485f4291c9a53 Mon Sep 17 00:00:00 2001 From: binwiederhier Date: Fri, 3 Mar 2023 14:55:37 -0500 Subject: [PATCH] EnableRateVisitor flag --- cmd/serve.go | 3 +++ server/config.go | 1 + server/server.go | 13 +++++++++---- server/server.yml | 14 ++++++++++++++ server/server_test.go | 13 +++++++++++-- 5 files changed, 38 insertions(+), 6 deletions(-) diff --git a/cmd/serve.go b/cmd/serve.go index c799f5a6..4e663024 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -63,6 +63,7 @@ var flagsServe = append( altsrc.NewBoolFlag(&cli.BoolFlag{Name: "enable-signup", Aliases: []string{"enable_signup"}, EnvVars: []string{"NTFY_ENABLE_SIGNUP"}, Value: false, Usage: "allows users to sign up via the web app, or API"}), altsrc.NewBoolFlag(&cli.BoolFlag{Name: "enable-login", Aliases: []string{"enable_login"}, EnvVars: []string{"NTFY_ENABLE_LOGIN"}, Value: false, Usage: "allows users to log in via the web app, or API"}), altsrc.NewBoolFlag(&cli.BoolFlag{Name: "enable-reservations", Aliases: []string{"enable_reservations"}, EnvVars: []string{"NTFY_ENABLE_RESERVATIONS"}, Value: false, Usage: "allows users to reserve topics (if their tier allows it)"}), + altsrc.NewBoolFlag(&cli.BoolFlag{Name: "enable-rate-visitor", Aliases: []string{"enable_rate_visitor"}, EnvVars: []string{"NTFY_ENABLE_RATE_VISITOR"}, Value: false, Usage: "enables subscriber-based rate limiting for UnifiedPush topics"}), altsrc.NewStringFlag(&cli.StringFlag{Name: "upstream-base-url", Aliases: []string{"upstream_base_url"}, EnvVars: []string{"NTFY_UPSTREAM_BASE_URL"}, Value: "", Usage: "forward poll request to an upstream server, this is needed for iOS push notifications for self-hosted servers"}), altsrc.NewStringFlag(&cli.StringFlag{Name: "smtp-sender-addr", Aliases: []string{"smtp_sender_addr"}, EnvVars: []string{"NTFY_SMTP_SENDER_ADDR"}, Usage: "SMTP server address (host:port) for outgoing emails"}), altsrc.NewStringFlag(&cli.StringFlag{Name: "smtp-sender-user", Aliases: []string{"smtp_sender_user"}, EnvVars: []string{"NTFY_SMTP_SENDER_USER"}, Usage: "SMTP user (if e-mail sending is enabled)"}), @@ -139,6 +140,7 @@ func execServe(c *cli.Context) error { enableSignup := c.Bool("enable-signup") enableLogin := c.Bool("enable-login") enableReservations := c.Bool("enable-reservations") + enableRateVisitor := c.Bool("enable-rate-visitor") upstreamBaseURL := c.String("upstream-base-url") smtpSenderAddr := c.String("smtp-sender-addr") smtpSenderUser := c.String("smtp-sender-user") @@ -312,6 +314,7 @@ func execServe(c *cli.Context) error { conf.EnableSignup = enableSignup conf.EnableLogin = enableLogin conf.EnableReservations = enableReservations + conf.EnableRateVisitor = enableRateVisitor conf.Version = c.App.Version // Set up hot-reloading of config diff --git a/server/config.go b/server/config.go index b29fb063..747cbf17 100644 --- a/server/config.go +++ b/server/config.go @@ -133,6 +133,7 @@ type Config struct { EnableSignup bool // Enable creation of accounts via API and UI EnableLogin bool EnableReservations bool // Allow users with role "user" to own/reserve topics + EnableRateVisitor bool // Enable subscriber-based rate limiting for UnifiedPush topics AccessControlAllowOrigin string // CORS header field to restrict access from web clients Version string // injected by App } diff --git a/server/server.go b/server/server.go index c1eab9b5..27f20478 100644 --- a/server/server.go +++ b/server/server.go @@ -597,7 +597,7 @@ func (s *Server) handlePublishWithoutResponse(r *http.Request, v *visitor) (*mes if e != nil { return nil, e.With(t) } - if unifiedpush && t.RateVisitor() == nil { + if unifiedpush && s.config.EnableRateVisitor && t.RateVisitor() == nil { // UnifiedPush clients must subscribe before publishing to allow proper subscriber-based rate limiting (see // Rate-Topics header). The 5xx response is because some app servers (in particular Mastodon) will remove // the subscription as invalid if any 400-499 code (except 429/408) is returned. @@ -1188,14 +1188,19 @@ func parseSubscribeParams(r *http.Request) (poll bool, since sinceMarker, schedu // maybeSetRateVisitors sets the rate visitor on a topic (v.SetRateVisitor), indicating that all messages published // to that topic will be rate limited against the rate visitor instead of the publishing visitor. // -// Setting the rate visitor is ony allowed if +// Setting the rate visitor is ony allowed if the `enable-rate-visitor` setting is enabled, AND // - auth-file is not set (everything is open by default) -// - the topic is reserved, and v.user is the owner -// - the topic is not reserved, and v.user has write access +// - or the topic is reserved, and v.user is the owner +// - or the topic is not reserved, and v.user has write access // // Note: This TEMPORARILY also registers all topics starting with "up" (= UnifiedPush). This is to ease the transition // until the Android app will send the "Rate-Topics" header. func (s *Server) maybeSetRateVisitors(r *http.Request, v *visitor, topics []*topic, rateTopics []string) error { + // Bail out if not enabled + if !s.config.EnableRateVisitor { + return nil + } + // Make a list of topics that we'll actually set the RateVisitor on eligibleRateTopics := make([]*topic, 0) for _, t := range topics { diff --git a/server/server.yml b/server/server.yml index 5c7955dd..8f4d646e 100644 --- a/server/server.yml +++ b/server/server.yml @@ -235,6 +235,20 @@ # visitor-attachment-total-size-limit: "100M" # visitor-attachment-daily-bandwidth-limit: "500M" +# Rate limiting: Enable subscriber-based rate limiting (mostly used for UnifiedPush) +# +# If enabled, subscribers may opt to have published messages counted against their own rate limits, as opposed +# to the publisher's rate limits. This is especially useful to increase the amount of messages that UnifiedPush +# publishers (e.g. Matrix/Mastodon servers) are allowed to send. +# +# Once enabled, a client may send a "Rate-Topics: ,,..." header when subscribing to topics via +# HTTP stream, or websockets, thereby registering itself as the "rate visitor", i.e. the visitor whose rate limits +# to use when publishing on this topic. +# +# For your home server, you likely DO NOT NEED THIS setting. +# +# enable-rate-visitors: false + # Payments integration via Stripe # # - stripe-secret-key is the key used for the Stripe API communication. Setting this values diff --git a/server/server_test.go b/server/server_test.go index 69237696..2dfb5643 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -15,6 +15,7 @@ import ( "net/netip" "os" "path/filepath" + "runtime/debug" "strings" "sync" "testing" @@ -1291,7 +1292,9 @@ func TestServer_MatrixGateway_Push_Success(t *testing.T) { } func TestServer_MatrixGateway_Push_Failure_NoSubscriber(t *testing.T) { - s := newTestServer(t, newTestConfig(t)) + c := newTestConfig(t) + c.EnableRateVisitor = true + s := newTestServer(t, c) notification := `{"notification":{"devices":[{"pushkey":"http://127.0.0.1:12345/mytopic?up=1"}]}}` response := request(t, s, "POST", "/_matrix/push/v1/notify", notification, nil) require.Equal(t, 507, response.Code) @@ -2029,6 +2032,7 @@ func TestServer_AnonymousUser_And_NonTierUser_Are_Same_Visitor(t *testing.T) { func TestServer_SubscriberRateLimiting_Success(t *testing.T) { c := newTestConfigWithAuthFile(t) c.VisitorRequestLimitBurst = 3 + c.EnableRateVisitor = true s := newTestServer(t, c) // "Register" visitor 1.2.3.4 to topic "subscriber1topic" as a rate limit visitor @@ -2082,6 +2086,7 @@ func TestServer_SubscriberRateLimiting_Success(t *testing.T) { func TestServer_SubscriberRateLimiting_UP_Only(t *testing.T) { c := newTestConfigWithAuthFile(t) c.VisitorRequestLimitBurst = 3 + c.EnableRateVisitor = true s := newTestServer(t, c) // "Register" 5 different UnifiedPush visitors @@ -2105,6 +2110,7 @@ func TestServer_SubscriberRateLimiting_UP_Only(t *testing.T) { func TestServer_Matrix_SubscriberRateLimiting_UP_Only(t *testing.T) { c := newTestConfig(t) c.VisitorRequestLimitBurst = 3 + c.EnableRateVisitor = true s := newTestServer(t, c) // "Register" 5 different UnifiedPush visitors @@ -2132,6 +2138,7 @@ func TestServer_Matrix_SubscriberRateLimiting_UP_Only(t *testing.T) { func TestServer_SubscriberRateLimiting_VisitorExpiration(t *testing.T) { c := newTestConfig(t) c.VisitorRequestLimitBurst = 3 + c.EnableRateVisitor = true s := newTestServer(t, c) // "Register" rate visitor @@ -2167,6 +2174,7 @@ func TestServer_SubscriberRateLimiting_VisitorExpiration(t *testing.T) { func TestServer_SubscriberRateLimiting_ProtectedTopics(t *testing.T) { c := newTestConfigWithAuthFile(t) c.AuthDefault = user.PermissionDenyAll + c.EnableRateVisitor = true s := newTestServer(t, c) // Create some ACLs @@ -2214,6 +2222,7 @@ func TestServer_SubscriberRateLimiting_ProtectedTopics(t *testing.T) { func TestServer_SubscriberRateLimiting_ProtectedTopics_WithDefaultReadWrite(t *testing.T) { c := newTestConfigWithAuthFile(t) c.AuthDefault = user.PermissionReadWrite + c.EnableRateVisitor = true s := newTestServer(t, c) // Create some ACLs @@ -2333,5 +2342,5 @@ func waitForWithMaxWait(t *testing.T, maxWait time.Duration, f func() bool) { } time.Sleep(100 * time.Millisecond) } - t.Fatalf("Function f did not succeed after %v", maxWait) + t.Fatalf("Function f did not succeed after %v: %s", maxWait, debug.Stack()) }