diff --git a/pkg/routes/rate_limit.go b/pkg/routes/rate_limit.go
index 8ef22ef56..9e899109a 100644
--- a/pkg/routes/rate_limit.go
+++ b/pkg/routes/rate_limit.go
@@ -41,10 +41,14 @@ func RateLimit(rateLimiter *limiter.Limiter, rateLimitKind string) echo.Middlewa
rateLimitKey = c.RealIP()
case "user":
auth, err := auth2.GetAuthFromClaims(c)
- if err != nil {
+ // Unauthenticated requests hit this middleware because v2 rate limits
+ // one group covering its public routes as well - key those by IP.
+ if err != nil || auth == nil {
log.Errorf("Error getting auth from jwt claims: %v", err)
+ rateLimitKey = "ip_" + c.RealIP()
+ } else {
+ rateLimitKey = "user_" + strconv.FormatInt(auth.GetID(), 10)
}
- rateLimitKey = "user_" + strconv.FormatInt(auth.GetID(), 10)
default:
log.Errorf("Unknown rate limit kind configured: %s", rateLimitKind)
}
diff --git a/pkg/routes/rate_limit_test.go b/pkg/routes/rate_limit_test.go
new file mode 100644
index 000000000..b1f4b4bb6
--- /dev/null
+++ b/pkg/routes/rate_limit_test.go
@@ -0,0 +1,120 @@
+// Vikunja is a to-do list application to facilitate your life.
+// Copyright 2018-present Vikunja and contributors. All rights reserved.
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program. If not, see .
+
+package routes
+
+import (
+ "net/http"
+ "net/http/httptest"
+ "testing"
+ "time"
+
+ "code.vikunja.io/api/pkg/log"
+ auth2 "code.vikunja.io/api/pkg/modules/auth"
+
+ "github.com/golang-jwt/jwt/v5"
+ "github.com/labstack/echo/v5"
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+ "github.com/ulule/limiter/v3"
+ "github.com/ulule/limiter/v3/drivers/store/memory"
+)
+
+func newRateLimitTestContext(e *echo.Echo, remoteAddr string) (*echo.Context, *httptest.ResponseRecorder) {
+ req := httptest.NewRequest(http.MethodGet, "/api/v2/info", nil)
+ req.RemoteAddr = remoteAddr
+ rec := httptest.NewRecorder()
+ return e.NewContext(req, rec), rec
+}
+
+func rateLimitTestHandler(rateLimitKind string, limit int64) (handler echo.HandlerFunc, called *int) {
+ calls := 0
+ rateLimiter := limiter.New(memory.NewStore(), limiter.Rate{Period: time.Minute, Limit: limit})
+ h := RateLimit(rateLimiter, rateLimitKind)(func(_ *echo.Context) error {
+ calls++
+ return nil
+ })
+ return h, &calls
+}
+
+// TestRateLimitUnauthenticated makes sure an unauthenticated request does not
+// panic when the rate limit kind is "user" - the v2 group rate limits its public
+// routes as well, so there are no jwt claims to key on.
+func TestRateLimitUnauthenticated(t *testing.T) {
+ log.InitLogger()
+ e := echo.New()
+
+ t.Run("does not panic and passes through", func(t *testing.T) {
+ h, calls := rateLimitTestHandler("user", 100)
+ c, rec := newRateLimitTestContext(e, "1.2.3.4:1234")
+
+ require.NotPanics(t, func() {
+ require.NoError(t, h(c))
+ })
+ assert.Equal(t, 1, *calls)
+ assert.Equal(t, "100", rec.Header().Get("X-RateLimit-Limit"))
+ assert.Equal(t, "99", rec.Header().Get("X-RateLimit-Remaining"))
+ })
+
+ t.Run("limits by ip", func(t *testing.T) {
+ h, calls := rateLimitTestHandler("user", 2)
+
+ for range 2 {
+ c, _ := newRateLimitTestContext(e, "1.2.3.4:1234")
+ require.NoError(t, h(c))
+ }
+
+ c, _ := newRateLimitTestContext(e, "1.2.3.4:1234")
+ err := h(c)
+ var he *echo.HTTPError
+ require.ErrorAs(t, err, &he)
+ assert.Equal(t, http.StatusTooManyRequests, he.Code)
+
+ // A different ip must have its own bucket, otherwise everyone shares one limit
+ other, _ := newRateLimitTestContext(e, "5.6.7.8:1234")
+ require.NoError(t, h(other))
+ assert.Equal(t, 3, *calls)
+ })
+}
+
+// TestRateLimitUser makes sure authenticated requests are still keyed by user id.
+func TestRateLimitUser(t *testing.T) {
+ log.InitLogger()
+ e := echo.New()
+
+ token := jwt.NewWithClaims(jwt.SigningMethodHS256, jwt.MapClaims{
+ "type": float64(auth2.AuthTypeUser),
+ "id": float64(42),
+ "username": "user42",
+ })
+
+ h, calls := rateLimitTestHandler("user", 2)
+
+ for range 2 {
+ c, _ := newRateLimitTestContext(e, "1.2.3.4:1234")
+ c.Set("user", token)
+ require.NoError(t, h(c))
+ }
+
+ // Same user from a different ip shares the bucket
+ c, _ := newRateLimitTestContext(e, "5.6.7.8:1234")
+ c.Set("user", token)
+ err := h(c)
+ var he *echo.HTTPError
+ require.ErrorAs(t, err, &he)
+ assert.Equal(t, http.StatusTooManyRequests, he.Code)
+ assert.Equal(t, 2, *calls)
+}
diff --git a/pkg/webtests/huma_rate_limit_test.go b/pkg/webtests/huma_rate_limit_test.go
new file mode 100644
index 000000000..b08867510
--- /dev/null
+++ b/pkg/webtests/huma_rate_limit_test.go
@@ -0,0 +1,60 @@
+// Vikunja is a to-do list application to facilitate your life.
+// Copyright 2018-present Vikunja and contributors. All rights reserved.
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program. If not, see .
+
+package webtests
+
+import (
+ "net/http"
+ "testing"
+
+ "code.vikunja.io/api/pkg/config"
+ "code.vikunja.io/api/pkg/routes"
+
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+)
+
+// TestHumaRateLimitUnauthenticated covers rate limiting with the default "user"
+// kind: v2 rate limits one group which also serves its public routes, so those
+// requests reach the middleware without any claims to key on.
+func TestHumaRateLimitUnauthenticated(t *testing.T) {
+ _, err := setupTestEnv()
+ require.NoError(t, err)
+
+ config.RateLimitEnabled.Set(true)
+ config.RateLimitKind.Set("user")
+ config.RateLimitLimit.Set(2)
+ defer func() {
+ config.RateLimitEnabled.Set(false)
+ config.RateLimitLimit.Set(100)
+ }()
+
+ e := routes.NewEcho()
+ routes.RegisterRoutes(e)
+
+ for _, path := range []string{"/api/v2/info", "/api/v2/health"} {
+ t.Run(path, func(t *testing.T) {
+ rec := humaRequest(t, e, http.MethodGet, path, "", "", "")
+ assert.NotEqual(t, http.StatusInternalServerError, rec.Code, "body: %s", rec.Body.String())
+ assert.Equal(t, "2", rec.Header().Get("X-RateLimit-Limit"))
+ })
+ }
+
+ t.Run("limits by ip", func(t *testing.T) {
+ rec := humaRequest(t, e, http.MethodGet, "/api/v2/info", "", "", "")
+ assert.Equal(t, http.StatusTooManyRequests, rec.Code, "body: %s", rec.Body.String())
+ })
+}