Anonymous: Fix anonymous cache ignoring device limit evaluation (#94218)

* ensure cache contains the evaluation result for device limit

* add device limit errors and warnings

* fix lint

pkg/services/anonymous/anonimpl/client.go

@@ -17,8 +17,9 @@ import (
 )
 
 var (
-	errInvalidOrg = errutil.Unauthorized("anonymous.invalid-org")
-	errInvalidID  = errutil.Unauthorized("anonymous.invalid-id")
+	errInvalidOrg  = errutil.Unauthorized("anonymous.invalid-org")
+	errInvalidID   = errutil.Unauthorized("anonymous.invalid-id")
+	errDeviceLimit = errutil.Unauthorized("anonymous.device-limit-reached", errutil.WithPublicMessage("Anonymous device limit reached. Contact Administrator"))
 )
 
 var _ authn.ContextAwareClient = new(Anonymous)
@@ -51,7 +52,7 @@ func (a *Anonymous) Authenticate(ctx context.Context, r *authn.Request) (*authn.
 
 	if err := a.anonDeviceService.TagDevice(ctx, httpReqCopy, anonymous.AnonDeviceUI); err != nil {
 		if errors.Is(err, anonstore.ErrDeviceLimitReached) {
-			return nil, err
+			return nil, errDeviceLimit.Errorf("limit reached for anonymous devices: %w", err)
 		}
 
 		a.log.Warn("Failed to tag anonymous session", "error", err)

pkg/services/anonymous/anonimpl/impl.go

@@ -2,6 +2,7 @@ package anonimpl
 
 import (
 	"context"
+	"errors"
 	"net/http"
 	"time"
 
@@ -79,20 +80,29 @@ func (a *AnonDeviceService) usageStatFn(ctx context.Context) (map[string]any, er
 	}, nil
 }
 
-func (a *AnonDeviceService) tagDeviceUI(ctx context.Context, httpReq *http.Request, device *anonstore.Device) error {
+func (a *AnonDeviceService) tagDeviceUI(ctx context.Context, device *anonstore.Device) error {
 	key := device.CacheKey()
 
-	if _, ok := a.localCache.Get(key); ok {
+	if val, ok := a.localCache.Get(key); ok {
+		if boolVal, ok := val.(bool); ok && !boolVal {
+			return anonstore.ErrDeviceLimitReached
+		}
 		return nil
 	}
 
-	a.localCache.SetDefault(key, struct{}{})
+	a.localCache.SetDefault(key, true)
 
 	if a.cfg.Env == setting.Dev {
 		a.log.Debug("Tagging device for UI", "deviceID", device.DeviceID, "device", device, "key", key)
 	}
 
 	if err := a.anonStore.CreateOrUpdateDevice(ctx, device); err != nil {
+		if errors.Is(err, anonstore.ErrDeviceLimitReached) {
+			a.localCache.SetDefault(key, false)
+			return err
+		}
+		// invalidate cache if there is an error
+		a.localCache.Delete(key)
 		return err
 	}
 
@@ -142,7 +152,7 @@ func (a *AnonDeviceService) TagDevice(ctx context.Context, httpReq *http.Request
 		UpdatedAt: time.Now(),
 	}
 
-	err = a.tagDeviceUI(ctx, httpReq, taggedDevice)
+	err = a.tagDeviceUI(ctx, taggedDevice)
 	if err != nil {
 		a.log.Debug("Failed to tag device for UI", "error", err)
 		return err

pkg/services/anonymous/anonimpl/impl_test.go

@@ -26,6 +26,10 @@ func TestMain(m *testing.M) {
 }
 
 func TestIntegrationDeviceService_tag(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping test in short mode")
+	}
+
 	type tagReq struct {
 		httpReq *http.Request
 		kind    anonymous.DeviceKind
@@ -152,6 +156,9 @@ func TestIntegrationDeviceService_tag(t *testing.T) {
 
 // Ensure that the local cache prevents request from being tagged
 func TestIntegrationAnonDeviceService_localCacheSafety(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping test in short mode")
+	}
 	store := db.InitTestDB(t)
 	anonService := ProvideAnonymousDeviceService(&usagestats.UsageStatsMock{},
 		&authntest.FakeService{}, store, setting.NewCfg(), orgtest.NewOrgServiceFake(), nil, actest.FakeAccessControl{}, &routing.RouteRegisterImpl{})
@@ -184,6 +191,10 @@ func TestIntegrationAnonDeviceService_localCacheSafety(t *testing.T) {
 }
 
 func TestIntegrationDeviceService_SearchDevice(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping test in short mode")
+	}
+
 	fixedTime := time.Date(2024, 1, 1, 12, 0, 0, 0, time.UTC) // Fixed timestamp for testing
 
 	testCases := []struct {
@@ -271,3 +282,88 @@ func TestIntegrationDeviceService_SearchDevice(t *testing.T) {
 		})
 	}
 }
+
+func TestIntegrationAnonDeviceService_DeviceLimitWithCache(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping test in short mode")
+	}
+	// Setup test environment
+	store := db.InitTestDB(t)
+	cfg := setting.NewCfg()
+	cfg.AnonymousDeviceLimit = 1 // Set device limit to 1 for testing
+	anonService := ProvideAnonymousDeviceService(
+		&usagestats.UsageStatsMock{},
+		&authntest.FakeService{},
+		store,
+		cfg,
+		orgtest.NewOrgServiceFake(),
+		nil,
+		actest.FakeAccessControl{},
+		&routing.RouteRegisterImpl{},
+	)
+
+	// Define test cases
+	testCases := []struct {
+		name        string
+		httpReq     *http.Request
+		expectedErr error
+	}{
+		{
+			name: "first request should succeed",
+			httpReq: &http.Request{
+				Header: http.Header{
+					"User-Agent":                            []string{"test"},
+					"X-Forwarded-For":                       []string{"10.30.30.1"},
+					http.CanonicalHeaderKey(deviceIDHeader): []string{"device1"},
+				},
+			},
+			expectedErr: nil,
+		},
+		{
+			name: "second request should fail due to device limit",
+			httpReq: &http.Request{
+				Header: http.Header{
+					"User-Agent":                            []string{"test"},
+					"X-Forwarded-For":                       []string{"10.30.30.2"},
+					http.CanonicalHeaderKey(deviceIDHeader): []string{"device2"},
+				},
+			},
+			expectedErr: anonstore.ErrDeviceLimitReached,
+		},
+		{
+			name: "repeat request should hit cache and succeed",
+			httpReq: &http.Request{
+				Header: http.Header{
+					"User-Agent":                            []string{"test"},
+					"X-Forwarded-For":                       []string{"10.30.30.1"},
+					http.CanonicalHeaderKey(deviceIDHeader): []string{"device1"},
+				},
+			},
+			expectedErr: nil,
+		},
+		{
+			name: "third request should hit cache and fail due to device limit",
+			httpReq: &http.Request{
+				Header: http.Header{
+					"User-Agent":                            []string{"test"},
+					"X-Forwarded-For":                       []string{"10.30.30.2"},
+					http.CanonicalHeaderKey(deviceIDHeader): []string{"device2"},
+				},
+			},
+			expectedErr: anonstore.ErrDeviceLimitReached,
+		},
+	}
+
+	// Run test cases
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			err := anonService.TagDevice(context.Background(), tc.httpReq, anonymous.AnonDeviceUI)
+			if tc.expectedErr != nil {
+				require.Error(t, err)
+				assert.Equal(t, tc.expectedErr, err)
+			} else {
+				require.NoError(t, err)
+			}
+		})
+	}
+}