Skip to content

Anonymous: Add configurable device limit #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ShashankFC wants to merge 1 commit into from
Closed

Anonymous: Add configurable device limit #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/grafana-data/src/types/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ export interface GrafanaConfig {
theme: GrafanaTheme;
theme2: GrafanaTheme2;
anonymousEnabled: boolean;
anonymousDeviceLimit: number | undefined;
featureToggles: FeatureToggles;
licenseInfo: LicenseInfo;
http2Enabled: boolean;
Expand Down
1 change: 1 addition & 0 deletions packages/grafana-runtime/src/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ export class GrafanaBootConfig implements GrafanaConfig {
theme2: GrafanaTheme2;
featureToggles: FeatureToggles = {};
anonymousEnabled = false;
anonymousDeviceLimit = undefined;
licenseInfo: LicenseInfo = {} as LicenseInfo;
rendererAvailable = false;
rendererVersion = '';
Expand Down
1 change: 1 addition & 0 deletions pkg/api/dtos/frontend_settings.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ type FrontendSettingsDTO struct {

FeatureToggles map[string]bool `json:"featureToggles"`
AnonymousEnabled bool `json:"anonymousEnabled"`
AnonymousDeviceLimit int64 `json:"anonymousDeviceLimit"`
RendererAvailable bool `json:"rendererAvailable"`
RendererVersion string `json:"rendererVersion"`
SecretsManagerPluginEnabled bool `json:"secretsManagerPluginEnabled"`
Expand Down
1 change: 1 addition & 0 deletions pkg/api/frontendsettings.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ func (hs *HTTPServer) getFrontendSettings(c *contextmodel.ReqContext) (*dtos.Fro

FeatureToggles: hs.Features.GetEnabled(c.Req.Context()),
AnonymousEnabled: hs.Cfg.AnonymousEnabled,
AnonymousDeviceLimit: hs.Cfg.AnonymousDeviceLimit,
RendererAvailable: hs.RenderService.IsAvailable(c.Req.Context()),
RendererVersion: hs.RenderService.Version(),
SecretsManagerPluginEnabled: secretsManagerPluginEnabled,
Expand Down
57 changes: 53 additions & 4 deletions pkg/services/anonymous/anonimpl/anonstore/database.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,14 @@ import (
)

const cacheKeyPrefix = "anon-device"
const anonymousDeviceExpiration = 30 * 24 * time.Hour

var ErrDeviceLimitReached = fmt.Errorf("device limit reached")

type AnonDBStore struct {
sqlStore db.DB
log log.Logger
sqlStore db.DB
log log.Logger
deviceLimit int64
}

type Device struct {
Expand Down Expand Up @@ -45,8 +49,8 @@ type AnonStore interface {
DeleteDevicesOlderThan(ctx context.Context, olderThan time.Time) error
}

func ProvideAnonDBStore(sqlStore db.DB) *AnonDBStore {
return &AnonDBStore{sqlStore: sqlStore, log: log.New("anonstore")}
func ProvideAnonDBStore(sqlStore db.DB, deviceLimit int64) *AnonDBStore {
return &AnonDBStore{sqlStore: sqlStore, log: log.New("anonstore"), deviceLimit: deviceLimit}
}

func (s *AnonDBStore) ListDevices(ctx context.Context, from *time.Time, to *time.Time) ([]*Device, error) {
Expand All @@ -65,9 +69,54 @@ func (s *AnonDBStore) ListDevices(ctx context.Context, from *time.Time, to *time
return devices, err
}

// updateDevice updates a device if it exists and has been updated between the given times.
func (s *AnonDBStore) updateDevice(ctx context.Context, device *Device) error {
const query = `UPDATE anon_device SET
client_ip = ?,
user_agent = ?,
updated_at = ?
WHERE device_id = ? AND updated_at BETWEEN ? AND ?`

args := []interface{}{device.ClientIP, device.UserAgent, device.UpdatedAt.UTC(), device.DeviceID,
device.UpdatedAt.UTC().Add(-anonymousDeviceExpiration), device.UpdatedAt.UTC().Add(time.Minute),
}
err := s.sqlStore.WithDbSession(ctx, func(dbSession *sqlstore.DBSession) error {
args = append([]interface{}{query}, args...)
result, err := dbSession.Exec(args...)
if err != nil {
return err
}

rowsAffected, err := result.RowsAffected()
if err != nil {
return err
}

if rowsAffected == 0 {
return ErrDeviceLimitReached
}

return nil
})

return err
}
Comment on lines +72 to +103

@coderabbitai coderabbitai Bot Jan 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Clarify error semantics: rowsAffected == 0 may not always indicate limit reached.

The updateDevice method returns ErrDeviceLimitReached when no rows are affected, but this could also occur if the device doesn't exist or if updated_at falls outside the time window for reasons other than the limit being reached. This conflates different failure modes.

Consider returning a more specific error or adding a check to distinguish between "device not found" and "device limit reached".

Suggested approach 
 		if rowsAffected == 0 {
-			return ErrDeviceLimitReached
+			return ErrDeviceLimitReached // Note: This also triggers if device doesn't exist within the expiration window
 		}

Alternatively, you could query for the device's existence first to provide a more accurate error, though this adds an extra query.


func (s *AnonDBStore) CreateOrUpdateDevice(ctx context.Context, device *Device) error {
var query string

// if device limit is reached, only update devices
if s.deviceLimit > 0 {
count, err := s.CountDevices(ctx, time.Now().UTC().Add(-anonymousDeviceExpiration), time.Now().UTC().Add(time.Minute))
if err != nil {
return err
}

if count >= s.deviceLimit {
return s.updateDevice(ctx, device)
}
}
Comment on lines +108 to +118

@coderabbitai coderabbitai Bot Jan 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's examine the full function containing lines 108-118
cd /root/repo && sed -n '80,130p' pkg/services/anonymous/anonimpl/anonstore/database.go

Repository: AI-Code-Review-Evals/coderabbit-grafana

Length of output: 143

🏁 Script executed:

# Let's also check the file size to ensure we can read it safely
wc -l pkg/services/anonymous/anonimpl/anonstore/database.go

Repository: AI-Code-Review-Evals/coderabbit-grafana

Length of output: 140

🏁 Script executed:

# Search for related comments about device limit enforcement
rg -i "device.*limit" pkg/services/anonymous/ -A 2 -B 2

Repository: AI-Code-Review-Evals/coderabbit-grafana

Length of output: 4253

🏁 Script executed:

# Look for transaction usage in this file
rg -i "transaction|begin|commit|rollback" pkg/services/anonymous/anonimpl/anonstore/database.go

Repository: AI-Code-Review-Evals/coderabbit-grafana

Length of output: 65

🏁 Script executed:

cat -n pkg/services/anonymous/anonimpl/anonstore/database.go

Repository: AI-Code-Review-Evals/coderabbit-grafana

Length of output: 7419

🏁 Script executed:

# Also check the test to understand expected behavior
cat -n pkg/services/anonymous/anonimpl/anonstore/database_test.go

Repository: AI-Code-Review-Evals/coderabbit-grafana

Length of output: 3519

TOCTOU race condition between count check and insert—device limit can be exceeded under concurrent load.

The count check (line 110) and subsequent insert/update operations (lines 115-155) are not atomic. Multiple concurrent requests can both pass the count check simultaneously and then both proceed to insert, exceeding the device limit. For example, with a limit of 1: Request A counts 0 devices, Request B counts 0 devices, then both insert, resulting in 2 devices.

Since ErrDeviceLimitReached is returned as an explicit error, this appears to be intended as a strict limit. To properly enforce this, wrap the count check and insert in a database transaction with row locking (e.g., SELECT ... FOR UPDATE) or use a unique constraint on a computed "active device slot" to prevent overcounting at the database level.


args := []any{device.DeviceID, device.ClientIP, device.UserAgent,
device.CreatedAt.UTC(), device.UpdatedAt.UTC()}
switch s.sqlStore.GetDBType() {
Expand Down
25 changes: 23 additions & 2 deletions pkg/services/anonymous/anonimpl/anonstore/database_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (

func TestIntegrationAnonStore_DeleteDevicesOlderThan(t *testing.T) {
store := db.InitTestDB(t)
anonDBStore := ProvideAnonDBStore(store)
anonDBStore := ProvideAnonDBStore(store, 0)
const keepFor = time.Hour * 24 * 61

anonDevice := &Device{
Expand Down Expand Up @@ -48,9 +48,30 @@ func TestIntegrationAnonStore_DeleteDevicesOlderThan(t *testing.T) {
assert.Equal(t, "keep", devices[0].DeviceID)
}

func TestIntegrationBeyondDeviceLimit(t *testing.T) {
store := db.InitTestDB(t)
anonDBStore := ProvideAnonDBStore(store, 1)

anonDevice := &Device{
DeviceID: "32mdo31deeqwes",
ClientIP: "10.30.30.2",
UserAgent: "test",
UpdatedAt: time.Now().Add(-time.Hour),
}

err := anonDBStore.CreateOrUpdateDevice(context.Background(), anonDevice)
require.NoError(t, err)

anonDevice.DeviceID = "keep"
anonDevice.UpdatedAt = time.Now().Add(-time.Hour)

err = anonDBStore.CreateOrUpdateDevice(context.Background(), anonDevice)
require.ErrorIs(t, err, ErrDeviceLimitReached)
}

func TestIntegrationAnonStore_DeleteDevice(t *testing.T) {
store := db.InitTestDB(t)
anonDBStore := ProvideAnonDBStore(store)
anonDBStore := ProvideAnonDBStore(store, 0)
const keepFor = time.Hour * 24 * 61

anonDevice := &Device{
Expand Down
6 changes: 2 additions & 4 deletions pkg/services/anonymous/anonimpl/api/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@ import (
"github.com/grafana/grafana/pkg/util"
)

const (
thirtyDays = 30 * 24 * time.Hour
)
const anonymousDeviceExpiration = 30 * 24 * time.Hour

type deviceDTO struct {
anonstore.Device
Expand Down Expand Up @@ -70,7 +68,7 @@ func (api *AnonDeviceServiceAPI) RegisterAPIEndpoints() {
// 404: notFoundError
// 500: internalServerError
func (api *AnonDeviceServiceAPI) ListDevices(c *contextmodel.ReqContext) response.Response {
fromTime := time.Now().Add(-thirtyDays)
fromTime := time.Now().Add(-anonymousDeviceExpiration)
toTime := time.Now()

devices, err := api.store.ListDevices(c.Req.Context(), &fromTime, &toTime)
Expand Down
23 changes: 8 additions & 15 deletions pkg/services/anonymous/anonimpl/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,20 @@ package anonimpl

import (
"context"
"errors"
"net/http"
"strings"
"time"

"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/services/anonymous"
"github.com/grafana/grafana/pkg/services/anonymous/anonimpl/anonstore"
"github.com/grafana/grafana/pkg/services/authn"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/setting"
)

var _ authn.ContextAwareClient = new(Anonymous)

const timeoutTag = 2 * time.Minute

type Anonymous struct {
cfg *setting.Cfg
log log.Logger
Expand All @@ -42,19 +41,13 @@ func (a *Anonymous) Authenticate(ctx context.Context, r *authn.Request) (*authn.
httpReqCopy.RemoteAddr = r.HTTPRequest.RemoteAddr
}

go func() {
defer func() {
if err := recover(); err != nil {
a.log.Warn("Tag anon session panic", "err", err)
}
}()

newCtx, cancel := context.WithTimeout(context.Background(), timeoutTag)
defer cancel()
if err := a.anonDeviceService.TagDevice(newCtx, httpReqCopy, anonymous.AnonDeviceUI); err != nil {
a.log.Warn("Failed to tag anonymous session", "error", err)
if err := a.anonDeviceService.TagDevice(ctx, httpReqCopy, anonymous.AnonDeviceUI); err != nil {
if errors.Is(err, anonstore.ErrDeviceLimitReached) {
return nil, err
}
}()

a.log.Warn("Failed to tag anonymous session", "error", err)
}

return &authn.Identity{
ID: authn.AnonymousNamespaceID,
Expand Down
8 changes: 5 additions & 3 deletions pkg/services/anonymous/anonimpl/impl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"time"

"github.com/grafana/grafana/pkg/api/routing"
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/localcache"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/infra/network"
Expand Down Expand Up @@ -33,13 +34,13 @@ type AnonDeviceService struct {
}

func ProvideAnonymousDeviceService(usageStats usagestats.Service, authBroker authn.Service,
anonStore anonstore.AnonStore, cfg *setting.Cfg, orgService org.Service,
sqlStore db.DB, cfg *setting.Cfg, orgService org.Service,
serverLockService *serverlock.ServerLockService, accesscontrol accesscontrol.AccessControl, routeRegister routing.RouteRegister,
) *AnonDeviceService {
a := &AnonDeviceService{
log: log.New("anonymous-session-service"),
localCache: localcache.New(29*time.Minute, 15*time.Minute),
anonStore: anonStore,
anonStore: anonstore.ProvideAnonDBStore(sqlStore, cfg.AnonymousDeviceLimit),
serverLock: serverLockService,
}

Expand All @@ -57,7 +58,7 @@ func ProvideAnonymousDeviceService(usageStats usagestats.Service, authBroker aut
authBroker.RegisterPostLoginHook(a.untagDevice, 100)
}

anonAPI := api.NewAnonDeviceServiceAPI(cfg, anonStore, accesscontrol, routeRegister)
anonAPI := api.NewAnonDeviceServiceAPI(cfg, a.anonStore, accesscontrol, routeRegister)
anonAPI.RegisterAPIEndpoints()

return a
Expand Down Expand Up @@ -143,6 +144,7 @@ func (a *AnonDeviceService) TagDevice(ctx context.Context, httpReq *http.Request
err = a.tagDeviceUI(ctx, httpReq, taggedDevice)
if err != nil {
a.log.Debug("Failed to tag device for UI", "error", err)
return err
}

return nil
Expand Down
8 changes: 3 additions & 5 deletions pkg/services/anonymous/anonimpl/impl_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,16 +113,15 @@ func TestIntegrationDeviceService_tag(t *testing.T) {
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
store := db.InitTestDB(t)
anonDBStore := anonstore.ProvideAnonDBStore(store)
anonService := ProvideAnonymousDeviceService(&usagestats.UsageStatsMock{},
&authntest.FakeService{}, anonDBStore, setting.NewCfg(), orgtest.NewOrgServiceFake(), nil, actest.FakeAccessControl{}, &routing.RouteRegisterImpl{})
&authntest.FakeService{}, store, setting.NewCfg(), orgtest.NewOrgServiceFake(), nil, actest.FakeAccessControl{}, &routing.RouteRegisterImpl{})

for _, req := range tc.req {
err := anonService.TagDevice(context.Background(), req.httpReq, req.kind)
require.NoError(t, err)
}

devices, err := anonDBStore.ListDevices(context.Background(), nil, nil)
devices, err := anonService.anonStore.ListDevices(context.Background(), nil, nil)
require.NoError(t, err)
require.Len(t, devices, int(tc.expectedAnonUICount))
if tc.expectedDevice != nil {
Expand All @@ -149,9 +148,8 @@ func TestIntegrationDeviceService_tag(t *testing.T) {
// Ensure that the local cache prevents request from being tagged
func TestIntegrationAnonDeviceService_localCacheSafety(t *testing.T) {
store := db.InitTestDB(t)
anonDBStore := anonstore.ProvideAnonDBStore(store)
anonService := ProvideAnonymousDeviceService(&usagestats.UsageStatsMock{},
&authntest.FakeService{}, anonDBStore, setting.NewCfg(), orgtest.NewOrgServiceFake(), nil, actest.FakeAccessControl{}, &routing.RouteRegisterImpl{})
&authntest.FakeService{}, store, setting.NewCfg(), orgtest.NewOrgServiceFake(), nil, actest.FakeAccessControl{}, &routing.RouteRegisterImpl{})

req := &http.Request{
Header: http.Header{
Expand Down
11 changes: 7 additions & 4 deletions pkg/setting/setting.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,7 @@ type Cfg struct {
AnonymousOrgName string
AnonymousOrgRole string
AnonymousHideVersion bool
AnonymousDeviceLimit int64

DateFormats DateFormats

Expand Down Expand Up @@ -1646,10 +1647,12 @@ func readAuthSettings(iniFile *ini.File, cfg *Cfg) (err error) {
readAuthGithubSettings(cfg)

// anonymous access
cfg.AnonymousEnabled = iniFile.Section("auth.anonymous").Key("enabled").MustBool(false)
cfg.AnonymousOrgName = valueAsString(iniFile.Section("auth.anonymous"), "org_name", "")
cfg.AnonymousOrgRole = valueAsString(iniFile.Section("auth.anonymous"), "org_role", "")
cfg.AnonymousHideVersion = iniFile.Section("auth.anonymous").Key("hide_version").MustBool(false)
anonSection := iniFile.Section("auth.anonymous")
cfg.AnonymousEnabled = anonSection.Key("enabled").MustBool(false)
cfg.AnonymousOrgName = valueAsString(anonSection, "org_name", "")
cfg.AnonymousOrgRole = valueAsString(anonSection, "org_role", "")
cfg.AnonymousHideVersion = anonSection.Key("hide_version").MustBool(false)
cfg.AnonymousDeviceLimit = anonSection.Key("device_limit").MustInt64(0)

// basic auth
authBasic := iniFile.Section("auth.basic")
Expand Down