Merge remote-tracking branch 'origin/main' into fix_debug_upload_url_from_mgmt

# Conflicts:
#	management/server/activity/codes.go
#	management/server/store/sql_store.go
#	management/server/store/sql_store_test.go
#	upload-server/server/server.go

management/server/http/handlers/groups/groups_handler.go

@@ -148,13 +148,10 @@ func (h *handler) updateGroup(w http.ResponseWriter, r *http.Request) {
 		peers = *req.Peers
 	}
 
-	resources := make([]types.Resource, 0)
-	if req.Resources != nil {
-		for _, res := range *req.Resources {
-			resource := types.Resource{}
-			resource.FromAPIRequest(&res)
-			resources = append(resources, resource)
-		}
+	resources, err := resourcesFromAPIRequest(req.Resources)
+	if err != nil {
+		util.WriteError(r.Context(), err, w)
+		return
 	}
 
 	group := types.Group{
@@ -210,13 +207,10 @@ func (h *handler) createGroup(w http.ResponseWriter, r *http.Request) {
 		peers = *req.Peers
 	}
 
-	resources := make([]types.Resource, 0)
-	if req.Resources != nil {
-		for _, res := range *req.Resources {
-			resource := types.Resource{}
-			resource.FromAPIRequest(&res)
-			resources = append(resources, resource)
-		}
+	resources, err := resourcesFromAPIRequest(req.Resources)
+	if err != nil {
+		util.WriteError(r.Context(), err, w)
+		return
 	}
 
 	group := types.Group{
@@ -335,11 +329,30 @@ func toGroupResponse(peers []*nbpeer.Peer, group *types.Group) *api.Group {
 	gr.PeersCount = len(gr.Peers)
 
 	for _, res := range group.Resources {
-		resResp := res.ToAPIResponse()
-		gr.Resources = append(gr.Resources, *resResp)
+		if resResp := res.ToAPIResponse(); resResp != nil {
+			gr.Resources = append(gr.Resources, *resResp)
+		}
 	}
 
 	gr.ResourcesCount = len(gr.Resources)
 
 	return &gr
 }
+
+func resourcesFromAPIRequest(req *[]api.Resource) ([]types.Resource, error) {
+	resources := make([]types.Resource, 0)
+	if req == nil {
+		return resources, nil
+	}
+
+	for _, res := range *req {
+		if res.Id == "" || !types.ResourceType(res.Type).Valid() {
+			return nil, status.Errorf(status.InvalidArgument, "resource id shouldn't be empty and type must be one of: peer, domain, host, subnet")
+		}
+		resource := types.Resource{}
+		resource.FromAPIRequest(&res)
+		resources = append(resources, resource)
+	}
+
+	return resources, nil
+}

management/server/http/handlers/groups/groups_handler_test.go

@@ -8,8 +8,8 @@ import (
 	"fmt"
 	"io"
 	"net/http"
-	"net/netip"
 	"net/http/httptest"
+	"net/netip"
 	"strings"
 	"testing"
 
@@ -208,6 +208,33 @@ func TestWriteGroup(t *testing.T) {
 			expectedStatus: http.StatusUnprocessableEntity,
 			expectedBody:   false,
 		},
+		{
+			name:        "Write Group POST Empty Resource",
+			requestType: http.MethodPost,
+			requestPath: "/api/groups",
+			requestBody: bytes.NewBuffer(
+				[]byte(`{"name":"With Resource","resources":[{}]}`)),
+			expectedStatus: http.StatusUnprocessableEntity,
+			expectedBody:   false,
+		},
+		{
+			name:        "Write Group PUT Empty Resource",
+			requestType: http.MethodPut,
+			requestPath: "/api/groups/id-existed",
+			requestBody: bytes.NewBuffer(
+				[]byte(`{"name":"With Resource","resources":[{"id":"","type":"host"}]}`)),
+			expectedStatus: http.StatusUnprocessableEntity,
+			expectedBody:   false,
+		},
+		{
+			name:        "Write Group POST Unknown Resource Type",
+			requestType: http.MethodPost,
+			requestPath: "/api/groups",
+			requestBody: bytes.NewBuffer(
+				[]byte(`{"name":"With Resource","resources":[{"id":"res-1","type":"banana"}]}`)),
+			expectedStatus: http.StatusUnprocessableEntity,
+			expectedBody:   false,
+		},
 		{
 			name:        "Write Group PUT OK",
 			requestType: http.MethodPut,
@@ -376,6 +403,20 @@ func TestGetAllGroups(t *testing.T) {
 	}
 }
 
+func TestToGroupResponseSkipsEmptyResource(t *testing.T) {
+	group := &types.Group{
+		ID:        "id-resources",
+		Name:      "Resources",
+		Issued:    types.GroupIssuedAPI,
+		Resources: []types.Resource{{}, {ID: "res-1", Type: types.ResourceTypeHost}},
+	}
+
+	got := toGroupResponse(nil, group)
+
+	assert.Equal(t, 1, got.ResourcesCount)
+	assert.Equal(t, []api.Resource{{Id: "res-1", Type: api.ResourceType(types.ResourceTypeHost)}}, got.Resources)
+}
+
 func TestDeleteGroup(t *testing.T) {
 	tt := []struct {
 		name           string

management/server/http/middleware/rate_limiter.go

@@ -14,12 +14,14 @@ import (
 	"golang.org/x/time/rate"
 
 	"github.com/netbirdio/netbird/shared/management/http/util"
+	"github.com/netbirdio/netbird/trustedproxy"
 )
 
 const (
-	RateLimitingEnabledEnv = "NB_API_RATE_LIMITING_ENABLED"
-	RateLimitingBurstEnv   = "NB_API_RATE_LIMITING_BURST"
-	RateLimitingRPMEnv     = "NB_API_RATE_LIMITING_RPM"
+	RateLimitingEnabledEnv        = "NB_API_RATE_LIMITING_ENABLED"
+	RateLimitingBurstEnv          = "NB_API_RATE_LIMITING_BURST"
+	RateLimitingRPMEnv            = "NB_API_RATE_LIMITING_RPM"
+	RateLimitingTrustedProxiesEnv = "NB_API_RATE_LIMITING_TRUSTED_PROXIES"
 
 	defaultAPIRPM   = 6
 	defaultAPIBurst = 500
@@ -35,6 +37,9 @@ type RateLimiterConfig struct {
 	CleanupInterval time.Duration
 	// LimiterTTL defines how long a limiter should be kept after last use (age threshold for removal)
 	LimiterTTL time.Duration
+	// TrustedProxies lists the upstream proxies whose forwarding headers may be
+	// believed. Empty means requests are keyed by their direct peer address.
+	TrustedProxies *trustedproxy.List
 }
 
 // DefaultRateLimiterConfig returns a default configuration
@@ -76,11 +81,18 @@ func RateLimiterConfigFromEnv() (cfg *RateLimiterConfig, enabled bool) {
 		burst = defaultAPIBurst
 	}
 
+	trusted, err := trustedproxy.Parse(os.Getenv(RateLimitingTrustedProxiesEnv))
+	if err != nil {
+		log.Warnf("parsing %s env var: %v, trusting no proxies", RateLimitingTrustedProxiesEnv, err)
+		trusted = nil
+	}
+
 	return &RateLimiterConfig{
 		RequestsPerMinute: float64(rpm),
 		Burst:             burst,
 		CleanupInterval:   6 * time.Hour,
 		LimiterTTL:        24 * time.Hour,
+		TrustedProxies:    trusted,
 	}, os.Getenv(RateLimitingEnabledEnv) == "true"
 }
 
@@ -250,7 +262,7 @@ func (rl *APIRateLimiter) Middleware(next http.Handler) http.Handler {
 			next.ServeHTTP(w, r)
 			return
 		}
-		clientIP := getClientIP(r)
+		clientIP := getClientIP(r, rl.config.TrustedProxies)
 		if !rl.Allow(clientIP) {
 			util.WriteErrorResponse("rate limit exceeded, please try again later", http.StatusTooManyRequests, w)
 			return
@@ -259,8 +271,15 @@ func (rl *APIRateLimiter) Middleware(next http.Handler) http.Handler {
 	})
 }
 
-// getClientIP extracts the client IP address from the request.
-func getClientIP(r *http.Request) string {
+// getClientIP extracts the client IP address from the request. Forwarding headers
+// are used only when the request arrives from a trusted proxy.
+func getClientIP(r *http.Request, trusted *trustedproxy.List) string {
+	if !trusted.Empty() {
+		if addr := trusted.ResolveClientIP(r.RemoteAddr, r.Header.Get("X-Forwarded-For")); addr.IsValid() {
+			return addr.String()
+		}
+	}
+
 	ip, _, err := net.SplitHostPort(r.RemoteAddr)
 	if err != nil {
 		return r.RemoteAddr

management/server/http/middleware/rate_limiter_test.go

@@ -9,6 +9,9 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/netbirdio/netbird/trustedproxy"
 )
 
 func TestAPIRateLimiter_Allow(t *testing.T) {
@@ -134,7 +137,7 @@ func TestGetClientIP(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			req := httptest.NewRequest(http.MethodGet, "/test", nil)
 			req.RemoteAddr = tc.remoteAddr
-			assert.Equal(t, tc.expected, getClientIP(req))
+			assert.Equal(t, tc.expected, getClientIP(req, nil))
 		})
 	}
 }
@@ -327,3 +330,47 @@ func TestRateLimiterConfigFromEnv(t *testing.T) {
 	assert.Equal(t, float64(defaultAPIRPM), cfg.RequestsPerMinute, "non-positive rpm must fall back to default")
 	assert.Equal(t, defaultAPIBurst, cfg.Burst, "non-positive burst must fall back to default")
 }
+
+func TestGetClientIP_TrustedProxies(t *testing.T) {
+	trusted, err := trustedproxy.Parse("10.0.0.0/8")
+	require.NoError(t, err)
+
+	tests := []struct {
+		name       string
+		list       *trustedproxy.List
+		remoteAddr string
+		xff        string
+		expected   string
+	}{
+		{
+			name:       "no trusted proxies ignores the header",
+			remoteAddr: "10.0.0.1:5555",
+			xff:        "1.1.1.1, 2.2.2.2",
+			expected:   "10.0.0.1",
+		},
+		{
+			name:       "behind a trusted proxy uses the right-most untrusted hop",
+			list:       trusted,
+			remoteAddr: "10.0.0.1:5555",
+			xff:        "1.1.1.1, 2.2.2.2",
+			expected:   "2.2.2.2",
+		},
+		{
+			name:       "a caller reaching us directly cannot forge the header",
+			list:       trusted,
+			remoteAddr: "203.0.113.5:5555",
+			xff:        "1.1.1.1",
+			expected:   "203.0.113.5",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			req := httptest.NewRequest(http.MethodGet, "/test", nil)
+			req.RemoteAddr = tc.remoteAddr
+			req.Header.Set("X-Forwarded-For", tc.xff)
+
+			assert.Equal(t, tc.expected, getClientIP(req, tc.list))
+		})
+	}
+}

management/server/http/testing/integration/events_handler_integration_test.go

@@ -3,6 +3,7 @@
 package integration
 
 import (
+	"context"
 	"encoding/json"
 	"net/http"
 	"net/http/httptest"
@@ -10,6 +11,7 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/netbirdio/netbird/management/server/http/testing/testing_tools"
 	"github.com/netbirdio/netbird/management/server/http/testing/testing_tools/channel"
@@ -34,7 +36,7 @@ func Test_Events_GetAll(t *testing.T) {
 
 	for _, user := range users {
 		t.Run(user.name+" - Get all events", func(t *testing.T) {
-			apiHandler, _, _ := channel.BuildApiBlackBoxWithDBState(t, "../testdata/events.sql", nil, false)
+			apiHandler, accountManager, _ := channel.BuildApiBlackBoxWithDBState(t, "../testdata/events.sql", nil, false)
 
 			// First, perform a mutation to generate an event (create a group as admin)
 			groupBody, err := json.Marshal(&api.GroupRequest{Name: "eventTestGroup"})
@@ -44,7 +46,14 @@ func Test_Events_GetAll(t *testing.T) {
 			createReq := testing_tools.BuildRequest(t, groupBody, http.MethodPost, "/api/groups", testing_tools.TestAdminId)
 			createRecorder := httptest.NewRecorder()
 			apiHandler.ServeHTTP(createRecorder, createReq)
-			assert.Equal(t, http.StatusOK, createRecorder.Code, "Failed to create group to generate event")
+			require.Equal(t, http.StatusOK, createRecorder.Code, "Failed to create group to generate event")
+
+			// Group creation returns before its asynchronous audit write finishes.
+			require.EventuallyWithT(t, func(c *assert.CollectT) {
+				events, err := accountManager.GetEvents(context.Background(), testing_tools.TestAccountId, testing_tools.TestAdminId)
+				assert.NoError(c, err)
+				assert.NotEmpty(c, events, "wait for the group creation event before checking permissions")
+			}, time.Second, 10*time.Millisecond)
 
 			// Now query events
 			req := testing_tools.BuildRequest(t, []byte{}, http.MethodGet, "/api/events", user.userId)