[management] add pat rate limiting (#4741)

2026-04-19 00:36:38 +00:00 · 2025-11-07 15:50:18 +01:00
parent 6aa4ba7af4
commit 48475ddc05
7 changed files with 496 additions and 4 deletions
--- a/management/server/http/middleware/auth_middleware_test.go
+++ b/management/server/http/middleware/auth_middleware_test.go
@@ -27,7 +27,9 @@ const (
 	domainCategory = "domainCategory"
 	userID         = "userID"
 	tokenID        = "tokenID"
+	tokenID2       = "tokenID2"
 	PAT            = "nbp_PAT"
+	PAT2           = "nbp_PAT2"
 	JWT            = "JWT"
 	wrongToken     = "wrongToken"
 )
@@ -49,6 +51,15 @@ var testAccount = &types.Account{
 					CreatedAt:      time.Now().UTC(),
 					LastUsed:       util.ToPtr(time.Now().UTC()),
 				},
+				tokenID2: {
+					ID:             tokenID2,
+					Name:           "My second token",
+					HashedToken:    "someHash2",
+					ExpirationDate: util.ToPtr(time.Now().UTC().AddDate(0, 0, 7)),
+					CreatedBy:      userID,
+					CreatedAt:      time.Now().UTC(),
+					LastUsed:       util.ToPtr(time.Now().UTC()),
+				},
 			},
 		},
 	},
@@ -58,6 +69,9 @@ func mockGetAccountInfoFromPAT(_ context.Context, token string) (user *types.Use
 	if token == PAT {
 		return testAccount.Users[userID], testAccount.Users[userID].PATs[tokenID], testAccount.Domain, testAccount.DomainCategory, nil
 	}
+	if token == PAT2 {
+		return testAccount.Users[userID], testAccount.Users[userID].PATs[tokenID2], testAccount.Domain, testAccount.DomainCategory, nil
+	}
 	return nil, nil, "", "", fmt.Errorf("PAT invalid")
 }

@@ -81,7 +95,7 @@ func mockValidateAndParseToken(_ context.Context, token string) (nbcontext.UserA
 }

 func mockMarkPATUsed(_ context.Context, token string) error {
-	if token == tokenID {
+	if token == tokenID || token == tokenID2 {
 		return nil
 	}
 	return fmt.Errorf("Should never get reached")
@@ -192,6 +206,7 @@ func TestAuthMiddleware_Handler(t *testing.T) {
 		func(ctx context.Context, userAuth nbcontext.UserAuth) (*types.User, error) {
 			return &types.User{}, nil
 		},
+		nil,
 	)

 	handlerToTest := authMiddleware.Handler(nextHandler)
@@ -221,6 +236,273 @@ func TestAuthMiddleware_Handler(t *testing.T) {
 	}
 }

+func TestAuthMiddleware_RateLimiting(t *testing.T) {
+	mockAuth := &auth.MockManager{
+		ValidateAndParseTokenFunc:       mockValidateAndParseToken,
+		EnsureUserAccessByJWTGroupsFunc: mockEnsureUserAccessByJWTGroups,
+		MarkPATUsedFunc:                 mockMarkPATUsed,
+		GetPATInfoFunc:                  mockGetAccountInfoFromPAT,
+	}
+
+	t.Run("PAT Token Rate Limiting - Burst Works", func(t *testing.T) {
+		// Configure rate limiter: 10 requests per minute with burst of 5
+		rateLimitConfig := &RateLimiterConfig{
+			RequestsPerMinute: 10,
+			Burst:             5,
+			CleanupInterval:   5 * time.Minute,
+			LimiterTTL:        10 * time.Minute,
+		}
+
+		authMiddleware := NewAuthMiddleware(
+			mockAuth,
+			func(ctx context.Context, userAuth nbcontext.UserAuth) (string, string, error) {
+				return userAuth.AccountId, userAuth.UserId, nil
+			},
+			func(ctx context.Context, userAuth nbcontext.UserAuth) error {
+				return nil
+			},
+			func(ctx context.Context, userAuth nbcontext.UserAuth) (*types.User, error) {
+				return &types.User{}, nil
+			},
+			rateLimitConfig,
+		)
+
+		handler := authMiddleware.Handler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(http.StatusOK)
+		}))
+
+		// Make burst requests - all should succeed
+		successCount := 0
+		for i := 0; i < 5; i++ {
+			req := httptest.NewRequest("GET", "http://testing/test", nil)
+			req.Header.Set("Authorization", "Token "+PAT)
+			rec := httptest.NewRecorder()
+
+			handler.ServeHTTP(rec, req)
+			if rec.Code == http.StatusOK {
+				successCount++
+			}
+		}
+
+		assert.Equal(t, 5, successCount, "All burst requests should succeed")
+
+		// The 6th request should fail (exceeded burst)
+		req := httptest.NewRequest("GET", "http://testing/test", nil)
+		req.Header.Set("Authorization", "Token "+PAT)
+		rec := httptest.NewRecorder()
+
+		handler.ServeHTTP(rec, req)
+		assert.Equal(t, http.StatusTooManyRequests, rec.Code, "Request beyond burst should be rate limited")
+	})
+
+	t.Run("PAT Token Rate Limiting - Rate Limit Enforced", func(t *testing.T) {
+		// Configure very low rate limit: 1 request per minute
+		rateLimitConfig := &RateLimiterConfig{
+			RequestsPerMinute: 1,
+			Burst:             1,
+			CleanupInterval:   5 * time.Minute,
+			LimiterTTL:        10 * time.Minute,
+		}
+
+		authMiddleware := NewAuthMiddleware(
+			mockAuth,
+			func(ctx context.Context, userAuth nbcontext.UserAuth) (string, string, error) {
+				return userAuth.AccountId, userAuth.UserId, nil
+			},
+			func(ctx context.Context, userAuth nbcontext.UserAuth) error {
+				return nil
+			},
+			func(ctx context.Context, userAuth nbcontext.UserAuth) (*types.User, error) {
+				return &types.User{}, nil
+			},
+			rateLimitConfig,
+		)
+
+		handler := authMiddleware.Handler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(http.StatusOK)
+		}))
+
+		// First request should succeed
+		req := httptest.NewRequest("GET", "http://testing/test", nil)
+		req.Header.Set("Authorization", "Token "+PAT)
+		rec := httptest.NewRecorder()
+		handler.ServeHTTP(rec, req)
+		assert.Equal(t, http.StatusOK, rec.Code, "First request should succeed")
+
+		// Second request should fail (rate limited)
+		req = httptest.NewRequest("GET", "http://testing/test", nil)
+		req.Header.Set("Authorization", "Token "+PAT)
+		rec = httptest.NewRecorder()
+		handler.ServeHTTP(rec, req)
+		assert.Equal(t, http.StatusTooManyRequests, rec.Code, "Second request should be rate limited")
+	})
+
+	t.Run("Bearer Token Not Rate Limited", func(t *testing.T) {
+		// Configure strict rate limit
+		rateLimitConfig := &RateLimiterConfig{
+			RequestsPerMinute: 1,
+			Burst:             1,
+			CleanupInterval:   5 * time.Minute,
+			LimiterTTL:        10 * time.Minute,
+		}
+
+		authMiddleware := NewAuthMiddleware(
+			mockAuth,
+			func(ctx context.Context, userAuth nbcontext.UserAuth) (string, string, error) {
+				return userAuth.AccountId, userAuth.UserId, nil
+			},
+			func(ctx context.Context, userAuth nbcontext.UserAuth) error {
+				return nil
+			},
+			func(ctx context.Context, userAuth nbcontext.UserAuth) (*types.User, error) {
+				return &types.User{}, nil
+			},
+			rateLimitConfig,
+		)
+
+		handler := authMiddleware.Handler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(http.StatusOK)
+		}))
+
+		// Make multiple requests with Bearer token - all should succeed
+		successCount := 0
+		for i := 0; i < 10; i++ {
+			req := httptest.NewRequest("GET", "http://testing/test", nil)
+			req.Header.Set("Authorization", "Bearer "+JWT)
+			rec := httptest.NewRecorder()
+
+			handler.ServeHTTP(rec, req)
+			if rec.Code == http.StatusOK {
+				successCount++
+			}
+		}
+
+		assert.Equal(t, 10, successCount, "All Bearer token requests should succeed (not rate limited)")
+	})
+
+	t.Run("PAT Token Rate Limiting Per Token", func(t *testing.T) {
+		// Configure rate limiter
+		rateLimitConfig := &RateLimiterConfig{
+			RequestsPerMinute: 1,
+			Burst:             1,
+			CleanupInterval:   5 * time.Minute,
+			LimiterTTL:        10 * time.Minute,
+		}
+
+		authMiddleware := NewAuthMiddleware(
+			mockAuth,
+			func(ctx context.Context, userAuth nbcontext.UserAuth) (string, string, error) {
+				return userAuth.AccountId, userAuth.UserId, nil
+			},
+			func(ctx context.Context, userAuth nbcontext.UserAuth) error {
+				return nil
+			},
+			func(ctx context.Context, userAuth nbcontext.UserAuth) (*types.User, error) {
+				return &types.User{}, nil
+			},
+			rateLimitConfig,
+		)
+
+		handler := authMiddleware.Handler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(http.StatusOK)
+		}))
+
+		// Use first PAT token
+		req := httptest.NewRequest("GET", "http://testing/test", nil)
+		req.Header.Set("Authorization", "Token "+PAT)
+		rec := httptest.NewRecorder()
+		handler.ServeHTTP(rec, req)
+		assert.Equal(t, http.StatusOK, rec.Code, "First request with PAT should succeed")
+
+		// Second request with same token should fail
+		req = httptest.NewRequest("GET", "http://testing/test", nil)
+		req.Header.Set("Authorization", "Token "+PAT)
+		rec = httptest.NewRecorder()
+		handler.ServeHTTP(rec, req)
+		assert.Equal(t, http.StatusTooManyRequests, rec.Code, "Second request with same PAT should be rate limited")
+
+		// Use second PAT token - should succeed because it has independent rate limit
+		req = httptest.NewRequest("GET", "http://testing/test", nil)
+		req.Header.Set("Authorization", "Token "+PAT2)
+		rec = httptest.NewRecorder()
+		handler.ServeHTTP(rec, req)
+		assert.Equal(t, http.StatusOK, rec.Code, "First request with PAT2 should succeed (independent rate limit)")
+
+		// Second request with PAT2 should also be rate limited
+		req = httptest.NewRequest("GET", "http://testing/test", nil)
+		req.Header.Set("Authorization", "Token "+PAT2)
+		rec = httptest.NewRecorder()
+		handler.ServeHTTP(rec, req)
+		assert.Equal(t, http.StatusTooManyRequests, rec.Code, "Second request with PAT2 should be rate limited")
+
+		// JWT should still work (not rate limited)
+		req = httptest.NewRequest("GET", "http://testing/test", nil)
+		req.Header.Set("Authorization", "Bearer "+JWT)
+		rec = httptest.NewRecorder()
+		handler.ServeHTTP(rec, req)
+		assert.Equal(t, http.StatusOK, rec.Code, "JWT request should succeed (not rate limited)")
+	})
+
+	t.Run("Rate Limiter Cleanup", func(t *testing.T) {
+		// Configure rate limiter with short cleanup interval and TTL for testing
+		rateLimitConfig := &RateLimiterConfig{
+			RequestsPerMinute: 60,
+			Burst:             1,
+			CleanupInterval:   100 * time.Millisecond,
+			LimiterTTL:        200 * time.Millisecond,
+		}
+
+		authMiddleware := NewAuthMiddleware(
+			mockAuth,
+			func(ctx context.Context, userAuth nbcontext.UserAuth) (string, string, error) {
+				return userAuth.AccountId, userAuth.UserId, nil
+			},
+			func(ctx context.Context, userAuth nbcontext.UserAuth) error {
+				return nil
+			},
+			func(ctx context.Context, userAuth nbcontext.UserAuth) (*types.User, error) {
+				return &types.User{}, nil
+			},
+			rateLimitConfig,
+		)
+
+		handler := authMiddleware.Handler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(http.StatusOK)
+		}))
+
+		// First request - should succeed
+		req := httptest.NewRequest("GET", "http://testing/test", nil)
+		req.Header.Set("Authorization", "Token "+PAT)
+		rec := httptest.NewRecorder()
+		handler.ServeHTTP(rec, req)
+		assert.Equal(t, http.StatusOK, rec.Code, "First request should succeed")
+
+		// Second request immediately - should fail (burst exhausted)
+		req = httptest.NewRequest("GET", "http://testing/test", nil)
+		req.Header.Set("Authorization", "Token "+PAT)
+		rec = httptest.NewRecorder()
+		handler.ServeHTTP(rec, req)
+		assert.Equal(t, http.StatusTooManyRequests, rec.Code, "Second request should be rate limited")
+
+		// Wait for limiter to be cleaned up (TTL + cleanup interval + buffer)
+		time.Sleep(400 * time.Millisecond)
+
+		// After cleanup, the limiter should be removed and recreated with full burst capacity
+		req = httptest.NewRequest("GET", "http://testing/test", nil)
+		req.Header.Set("Authorization", "Token "+PAT)
+		rec = httptest.NewRecorder()
+		handler.ServeHTTP(rec, req)
+		assert.Equal(t, http.StatusOK, rec.Code, "Request after cleanup should succeed (new limiter with full burst)")
+
+		// Verify it's a fresh limiter by checking burst is reset
+		req = httptest.NewRequest("GET", "http://testing/test", nil)
+		req.Header.Set("Authorization", "Token "+PAT)
+		rec = httptest.NewRecorder()
+		handler.ServeHTTP(rec, req)
+		assert.Equal(t, http.StatusTooManyRequests, rec.Code, "Second request after cleanup should be rate limited again")
+	})
+}
+
 func TestAuthMiddleware_Handler_Child(t *testing.T) {
 	tt := []struct {
 		name             string
@@ -297,6 +579,7 @@ func TestAuthMiddleware_Handler_Child(t *testing.T) {
 		func(ctx context.Context, userAuth nbcontext.UserAuth) (*types.User, error) {
 			return &types.User{}, nil
 		},
+		nil,
 	)

 	for _, tc := range tt {