Skip to content

Anonymous: Add configurable device limit #10

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

Open
akshayutture-augment wants to merge 1 commit into
base: enhance-anonymous-access
Choose a base branch
from
Open

Anonymous: Add configurable device limit #10

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),
Comment on lines +73 to +81
Copy link

@greptile-apps greptile-apps bot Nov 14, 2025

Choose a reason for hiding this comment

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

logic: Flawed logic: updateDevice uses time-based condition that will fail for most update attempts. When device.UpdatedAt is "now" and you check updated_at BETWEEN (now - 30 days) AND (now + 1 min), it only works if DB already has a recent entry. New devices or old devices won't match.

The WHERE clause should only check device_id:

Suggested change
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),
const query = `UPDATE anon_device SET
client_ip = ?,
user_agent = ?,
updated_at = ?
WHERE device_id = ?`
args := []interface{}{device.ClientIP, device.UserAgent, device.UpdatedAt.UTC(), device.DeviceID}
Prompt To Fix With AI 
This is a comment left during a code review.
Path: pkg/services/anonymous/anonimpl/anonstore/database.go
Line: 73:81

Comment:
**logic:** Flawed logic: `updateDevice` uses time-based condition that will fail for most update attempts. When `device.UpdatedAt` is "now" and you check `updated_at BETWEEN (now - 30 days) AND (now + 1 min)`, it only works if DB already has a recent entry. New devices or old devices won't match.

The WHERE clause should only check `device_id`:

```suggestion
	const query = `UPDATE anon_device SET
client_ip = ?,
user_agent = ?,
updated_at = ?
WHERE device_id = ?`

	args := []interface{}{device.ClientIP, device.UserAgent, device.UpdatedAt.UTC(), device.DeviceID}
```

How can I resolve this? If you propose a fix, please make it concise.

}
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
}

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 +109 to +117
Copy link

@greptile-apps greptile-apps bot Nov 14, 2025

Choose a reason for hiding this comment

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

logic: Race condition: concurrent requests can bypass limit. Between CountDevices and updateDevice, multiple goroutines can see count < limit and all proceed. Use database-level locking or atomic operations.

Suggested change
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)
}
// if device limit is reached, only update devices
if s.deviceLimit > 0 {
// Try update first - if it succeeds, device exists
err := s.updateDevice(ctx, device)
if err == nil {
return nil
}
if !errors.Is(err, ErrDeviceLimitReached) {
return err
}
// Device doesn't exist - check limit with lock or use constraint
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 ErrDeviceLimitReached
}
}
Prompt To Fix With AI 
This is a comment left during a code review.
Path: pkg/services/anonymous/anonimpl/anonstore/database.go
Line: 109:117

Comment:
**logic:** Race condition: concurrent requests can bypass limit. Between `CountDevices` and `updateDevice`, multiple goroutines can see count &lt; limit and all proceed. Use database-level locking or atomic operations.

```suggestion
	// if device limit is reached, only update devices
	if s.deviceLimit > 0 {
		// Try update first - if it succeeds, device exists
		err := s.updateDevice(ctx, device)
		if err == nil {
			return nil
		}
		if !errors.Is(err, ErrDeviceLimitReached) {
			return err
		}
		
		// Device doesn't exist - check limit with lock or use constraint
		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 ErrDeviceLimitReached
		}
	}
```

How can I resolve this? If you propose a fix, please make it concise.

}

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)
}
Comment on lines +44 to +50
Copy link

@greptile-apps greptile-apps bot Nov 14, 2025

Choose a reason for hiding this comment

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

style: Changed from async to blocking call without timeout. If TagDevice is slow (DB contention during high traffic), authentication hangs. The old code had a 2-minute timeout.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: pkg/services/anonymous/anonimpl/client.go
Line: 44:50

Comment:
**style:** Changed from async to blocking call without timeout. If `TagDevice` is slow (DB contention during high traffic), authentication hangs. The old code had a 2-minute timeout.

How can I resolve this? If you propose a fix, please make it concise.


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