diff --git a/pkg/registry/backgroundsvcs/background_services.go b/pkg/registry/backgroundsvcs/background_services.go index a71835837dd..ec2425c8402 100644 --- a/pkg/registry/backgroundsvcs/background_services.go +++ b/pkg/registry/backgroundsvcs/background_services.go @@ -9,6 +9,7 @@ import ( "github.com/grafana/grafana/pkg/infra/usagestats/statscollector" "github.com/grafana/grafana/pkg/registry" "github.com/grafana/grafana/pkg/services/alerting" + "github.com/grafana/grafana/pkg/services/anonymous/anonimpl" "github.com/grafana/grafana/pkg/services/auth" "github.com/grafana/grafana/pkg/services/cleanup" "github.com/grafana/grafana/pkg/services/dashboardsnapshots" @@ -54,6 +55,7 @@ func ProvideBackgroundServiceRegistry( publicDashboardsMetric *publicdashboardsmetric.Service, keyRetriever *dynamic.KeyRetriever, dynamicAngularDetectorsProvider *angulardetectorsprovider.Dynamic, + anon *anonimpl.AnonDeviceService, // Need to make sure these are initialized, is there a better place to put them? _ dashboardsnapshots.Service, _ *alerting.AlertNotificationService, _ serviceaccounts.Service, _ *guardian.Provider, @@ -92,6 +94,7 @@ func ProvideBackgroundServiceRegistry( publicDashboardsMetric, keyRetriever, dynamicAngularDetectorsProvider, + anon, ) } diff --git a/pkg/server/wire.go b/pkg/server/wire.go index ee79f4de4ab..75c4f8fbdd7 100644 --- a/pkg/server/wire.go +++ b/pkg/server/wire.go @@ -8,6 +8,7 @@ package server import ( "github.com/google/wire" + sdkhttpclient "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient" "github.com/grafana/grafana/pkg/api" @@ -39,6 +40,7 @@ import ( "github.com/grafana/grafana/pkg/services/alerting" "github.com/grafana/grafana/pkg/services/annotations" "github.com/grafana/grafana/pkg/services/annotations/annotationsimpl" + "github.com/grafana/grafana/pkg/services/anonymous/anonimpl/anonstore" "github.com/grafana/grafana/pkg/services/apikey/apikeyimpl" "github.com/grafana/grafana/pkg/services/auth/jwt" "github.com/grafana/grafana/pkg/services/authn/authnimpl" @@ -351,6 +353,8 @@ var wireBasicSet = wire.NewSet( authnimpl.ProvideAuthnService, supportbundlesimpl.ProvideService, oasimpl.ProvideService, + anonstore.ProvideAnonDBStore, + wire.Bind(new(anonstore.AnonStore), new(*anonstore.AnonDBStore)), wire.Bind(new(oauthserver.OAuth2Server), new(*oasimpl.OAuth2ServiceImpl)), loggermw.Provide, signingkeysimpl.ProvideEmbeddedSigningKeysService, diff --git a/pkg/services/anonymous/anonimpl/anonstore/database.go b/pkg/services/anonymous/anonimpl/anonstore/database.go new file mode 100644 index 00000000000..f0d3267aa26 --- /dev/null +++ b/pkg/services/anonymous/anonimpl/anonstore/database.go @@ -0,0 +1,126 @@ +package anonstore + +import ( + "context" + "fmt" + "strings" + "time" + + "github.com/grafana/grafana/pkg/infra/db" + "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/services/sqlstore/migrator" +) + +const cacheKeyPrefix = "anon-device" + +type AnonDBStore struct { + sqlStore db.DB + log log.Logger +} + +type Device struct { + ID int64 `json:"-" db:"id"` + DeviceID string `json:"device_id" db:"device_id"` + ClientIP string `json:"client_ip" db:"client_ip"` + UserAgent string `json:"user_agent" db:"user_agent"` + CreatedAt time.Time `json:"created_at" db:"created_at"` + UpdatedAt time.Time `json:"updated_at" db:"updated_at"` +} + +func (a *Device) CacheKey() string { + return strings.Join([]string{cacheKeyPrefix, a.DeviceID}, ":") +} + +type AnonStore interface { + // ListDevices returns all devices that have been updated between the given times. + ListDevices(ctx context.Context, from *time.Time, to *time.Time) ([]*Device, error) + // CreateOrUpdateDevice creates or updates a device. + CreateOrUpdateDevice(ctx context.Context, device *Device) error + // CountDevices returns the number of devices that have been updated between the given times. + CountDevices(ctx context.Context, from time.Time, to time.Time) (int64, error) + // DeleteDevice deletes a device by its ID. + DeleteDevice(ctx context.Context, deviceID string) error + // DeleteDevicesOlderThan deletes all devices that have no been updated since the given time. + DeleteDevicesOlderThan(ctx context.Context, olderThan time.Time) error +} + +func ProvideAnonDBStore(sqlStore db.DB) *AnonDBStore { + return &AnonDBStore{sqlStore: sqlStore, log: log.New("anonstore")} +} + +func (s *AnonDBStore) ListDevices(ctx context.Context, from *time.Time, to *time.Time) ([]*Device, error) { + devices := []*Device{} + query := "SELECT * FROM anon_device" + args := []any{} + if from != nil && to != nil { + query += " WHERE updated_at BETWEEN ? AND ?" + args = append(args, from.UTC(), to.UTC()) + } + err := s.sqlStore.GetSqlxSession().Select(ctx, &devices, query, args...) + if err != nil { + return nil, err + } + return devices, nil +} + +func (s *AnonDBStore) CreateOrUpdateDevice(ctx context.Context, device *Device) error { + var query string + + args := []any{device.DeviceID, device.ClientIP, device.UserAgent, + device.CreatedAt.UTC(), device.UpdatedAt.UTC()} + switch s.sqlStore.GetDBType() { + case migrator.Postgres: + query = `INSERT INTO anon_device (device_id, client_ip, user_agent, created_at, updated_at) +VALUES ($1, $2, $3, $4, $5) +ON CONFLICT (device_id) DO UPDATE SET +client_ip = $2, +user_agent = $3, +updated_at = $5 +RETURNING id` + case migrator.MySQL: + query = `INSERT INTO anon_device (device_id, client_ip, user_agent, created_at, updated_at) +VALUES (?, ?, ?, ?, ?) +ON DUPLICATE KEY UPDATE +client_ip = VALUES(client_ip), +user_agent = VALUES(user_agent), +updated_at = VALUES(updated_at)` + case migrator.SQLite: + query = `INSERT INTO anon_device (device_id, client_ip, user_agent, created_at, updated_at) +VALUES (?, ?, ?, ?, ?) +ON CONFLICT (device_id) DO UPDATE SET +client_ip = excluded.client_ip, +user_agent = excluded.user_agent, +updated_at = excluded.updated_at` + default: + return fmt.Errorf("unsupported database driver: %s", s.sqlStore.GetDBType()) + } + + _, err := s.sqlStore.GetSqlxSession().Exec(ctx, query, args...) + return err +} + +func (s *AnonDBStore) CountDevices(ctx context.Context, from time.Time, to time.Time) (int64, error) { + var count int64 + err := s.sqlStore.GetSqlxSession().Get(ctx, &count, "SELECT COUNT(*) FROM anon_device WHERE updated_at BETWEEN ? AND ?", from.UTC(), to.UTC()) + if err != nil { + return 0, err + } + return count, nil +} + +func (s *AnonDBStore) DeleteDevice(ctx context.Context, deviceID string) error { + _, err := s.sqlStore.GetSqlxSession().Exec(ctx, "DELETE FROM anon_device WHERE device_id = ?", deviceID) + if err != nil { + return err + } + return nil +} + +// deleteOldDevices deletes all devices that have no been updated since the given time. +func (s *AnonDBStore) DeleteDevicesOlderThan(ctx context.Context, olderThan time.Time) error { + _, err := s.sqlStore.GetSqlxSession().Exec(ctx, "DELETE FROM anon_device WHERE updated_at <= ?", olderThan.UTC()) + if err != nil { + return err + } + return nil +} diff --git a/pkg/services/anonymous/anonimpl/anonstore/database_test.go b/pkg/services/anonymous/anonimpl/anonstore/database_test.go new file mode 100644 index 00000000000..b94dd82d286 --- /dev/null +++ b/pkg/services/anonymous/anonimpl/anonstore/database_test.go @@ -0,0 +1,79 @@ +package anonstore + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/grafana/grafana/pkg/infra/db" +) + +func TestIntegrationAnonStore_DeleteDevicesOlderThan(t *testing.T) { + store := db.InitTestDB(t) + anonDBStore := ProvideAnonDBStore(store) + const keepFor = time.Hour * 24 * 61 + + anonDevice := &Device{ + DeviceID: "32mdo31deeqwes", + ClientIP: "10.30.30.2", + UserAgent: "test", + UpdatedAt: time.Now().Add(-keepFor).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.NoError(t, err) + + from := time.Now().Add(-2 * keepFor) + to := time.Now() + + count, err := anonDBStore.CountDevices(context.Background(), from, to) + require.NoError(t, err) + require.Equal(t, int64(2), count) + + err = anonDBStore.DeleteDevicesOlderThan(context.Background(), time.Now().Add(-keepFor)) + require.NoError(t, err) + + devices, err := anonDBStore.ListDevices(context.Background(), &from, &to) + require.NoError(t, err) + require.Equal(t, 1, len(devices)) + assert.Equal(t, "keep", devices[0].DeviceID) +} + +func TestIntegrationAnonStore_DeleteDevice(t *testing.T) { + store := db.InitTestDB(t) + anonDBStore := ProvideAnonDBStore(store) + const keepFor = time.Hour * 24 * 61 + + anonDevice := &Device{ + DeviceID: "32mdo31deeqwes", + ClientIP: "10.30.30.2", + UserAgent: "test", + UpdatedAt: time.Now().Add(-keepFor).Add(-time.Hour), + } + + err := anonDBStore.CreateOrUpdateDevice(context.Background(), anonDevice) + require.NoError(t, err) + + from := time.Now().Add(-2 * keepFor) + to := time.Now() + + count, err := anonDBStore.CountDevices(context.Background(), from, to) + require.NoError(t, err) + require.Equal(t, int64(1), count) + + err = anonDBStore.DeleteDevice(context.Background(), "32mdo31deeqwes") + require.NoError(t, err) + + devices, err := anonDBStore.ListDevices(context.Background(), &from, &to) + require.NoError(t, err) + require.Equal(t, 0, len(devices)) +} diff --git a/pkg/services/anonymous/anonimpl/anonstore/fake.go b/pkg/services/anonymous/anonimpl/anonstore/fake.go new file mode 100644 index 00000000000..9c7249d37bf --- /dev/null +++ b/pkg/services/anonymous/anonimpl/anonstore/fake.go @@ -0,0 +1,21 @@ +package anonstore + +import ( + "context" + "time" +) + +type FakeAnonStore struct { +} + +func (s *FakeAnonStore) ListDevices(ctx context.Context, from *time.Time, to *time.Time) ([]*Device, error) { + return nil, nil +} + +func (s *FakeAnonStore) CreateOrUpdateDevice(ctx context.Context, device *Device) error { + return nil +} + +func (s *FakeAnonStore) CountDevices(ctx context.Context, from time.Time, to time.Time) (int64, error) { + return 0, nil +} diff --git a/pkg/services/authn/clients/anonymous.go b/pkg/services/anonymous/anonimpl/client.go similarity index 87% rename from pkg/services/authn/clients/anonymous.go rename to pkg/services/anonymous/anonimpl/client.go index 92c4190bcbd..8035272a9ce 100644 --- a/pkg/services/authn/clients/anonymous.go +++ b/pkg/services/anonymous/anonimpl/client.go @@ -1,4 +1,4 @@ -package clients +package anonimpl import ( "context" @@ -17,15 +17,6 @@ var _ authn.ContextAwareClient = new(Anonymous) const timeoutTag = 2 * time.Minute -func ProvideAnonymous(cfg *setting.Cfg, orgService org.Service, anonDeviceService anonymous.Service) *Anonymous { - return &Anonymous{ - cfg: cfg, - log: log.New("authn.anonymous"), - orgService: orgService, - anonDeviceService: anonDeviceService, - } -} - type Anonymous struct { cfg *setting.Cfg log log.Logger @@ -60,7 +51,7 @@ func (a *Anonymous) Authenticate(ctx context.Context, r *authn.Request) (*authn. newCtx, cancel := context.WithTimeout(context.Background(), timeoutTag) defer cancel() - if err := a.anonDeviceService.TagDevice(newCtx, httpReqCopy, anonymous.AnonDevice); err != nil { + if err := a.anonDeviceService.TagDevice(newCtx, httpReqCopy, anonymous.AnonDeviceUI); err != nil { a.log.Warn("Failed to tag anonymous session", "error", err) } }() diff --git a/pkg/services/authn/clients/anonymous_test.go b/pkg/services/anonymous/anonimpl/client_test.go similarity index 99% rename from pkg/services/authn/clients/anonymous_test.go rename to pkg/services/anonymous/anonimpl/client_test.go index 724dfbd2dc2..ab722a379ff 100644 --- a/pkg/services/authn/clients/anonymous_test.go +++ b/pkg/services/anonymous/anonimpl/client_test.go @@ -1,4 +1,4 @@ -package clients +package anonimpl import ( "context" diff --git a/pkg/services/anonymous/anonimpl/impl.go b/pkg/services/anonymous/anonimpl/impl.go index 2902fa3a1ce..fec95479aab 100644 --- a/pkg/services/anonymous/anonimpl/impl.go +++ b/pkg/services/anonymous/anonimpl/impl.go @@ -2,144 +2,76 @@ package anonimpl import ( "context" - "encoding/hex" - "encoding/json" - "fmt" - "hash/fnv" "net/http" - "strings" "time" "github.com/grafana/grafana/pkg/infra/localcache" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/network" - "github.com/grafana/grafana/pkg/infra/remotecache" + "github.com/grafana/grafana/pkg/infra/serverlock" "github.com/grafana/grafana/pkg/infra/usagestats" "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" "github.com/grafana/grafana/pkg/web" ) const thirtyDays = 30 * 24 * time.Hour const deviceIDHeader = "X-Grafana-Device-Id" - -type Device struct { - Kind anonymous.DeviceKind `json:"kind"` - IP string `json:"ip"` - UserAgent string `json:"user_agent"` - LastSeen time.Time `json:"last_seen"` -} - -func (a *Device) Key() (string, error) { - key := strings.Builder{} - key.WriteString(a.IP) - key.WriteString(a.UserAgent) - - hash := fnv.New128a() - if _, err := hash.Write([]byte(key.String())); err != nil { - return "", fmt.Errorf("failed to write to hash: %w", err) - } - - return strings.Join([]string{string(a.Kind), hex.EncodeToString(hash.Sum(nil))}, ":"), nil -} - -func (a *Device) UIKey(deviceID string) (string, error) { - return strings.Join([]string{string(a.Kind), deviceID}, ":"), nil -} +const keepFor = time.Hour * 24 * 61 type AnonDeviceService struct { - remoteCache remotecache.CacheStorage - log log.Logger - localCache *localcache.CacheService + log log.Logger + localCache *localcache.CacheService + anonStore anonstore.AnonStore + serverLock *serverlock.ServerLockService } -func ProvideAnonymousDeviceService(remoteCache remotecache.CacheStorage, usageStats usagestats.Service) *AnonDeviceService { +func ProvideAnonymousDeviceService(usageStats usagestats.Service, authBroker authn.Service, + anonStore anonstore.AnonStore, cfg *setting.Cfg, orgService org.Service, + serverLockService *serverlock.ServerLockService, +) *AnonDeviceService { a := &AnonDeviceService{ - remoteCache: remoteCache, - log: log.New("anonymous-session-service"), - localCache: localcache.New(29*time.Minute, 15*time.Minute), + log: log.New("anonymous-session-service"), + localCache: localcache.New(29*time.Minute, 15*time.Minute), + anonStore: anonStore, + serverLock: serverLockService, } usageStats.RegisterMetricsFunc(a.usageStatFn) + anonClient := &Anonymous{ + cfg: cfg, + log: log.New("authn.anonymous"), + orgService: orgService, + anonDeviceService: a, + } + + if anonClient.cfg.AnonymousEnabled { + authBroker.RegisterClient(anonClient) + authBroker.RegisterPostLoginHook(a.untagDevice, 100) + } + return a } func (a *AnonDeviceService) usageStatFn(ctx context.Context) (map[string]any, error) { - anonDeviceCount, err := a.remoteCache.Count(ctx, string(anonymous.AnonDevice)) + // Count the number of unique devices that have been updated in the last 30 days. + // One minute is added to the end time as mysql has a precision of seconds and it will break tests that write too fast. + anonUIDeviceCount, err := a.anonStore.CountDevices(ctx, time.Now().Add(-thirtyDays), time.Now().Add(time.Minute)) if err != nil { - return nil, nil - } - - authedDeviceCount, err := a.remoteCache.Count(ctx, string(anonymous.AuthedDevice)) - if err != nil { - return nil, nil - } - - anonUIDeviceCount, err := a.remoteCache.Count(ctx, string(anonymous.AnonDeviceUI)) - if err != nil { - return nil, nil - } - - authedUIDeviceCount, err := a.remoteCache.Count(ctx, string(anonymous.AuthedDeviceUI)) - if err != nil { - return nil, nil + return nil, err } return map[string]any{ - "stats.anonymous.session.count": anonDeviceCount, // keep session for legacy data - "stats.users.device.count": authedDeviceCount, "stats.anonymous.device.ui.count": anonUIDeviceCount, - "stats.users.device.ui.count": authedUIDeviceCount, }, nil } -func (a *AnonDeviceService) untagDevice(ctx context.Context, device *Device) error { - key, err := device.Key() - if err != nil { - return err - } - - if err := a.remoteCache.Delete(ctx, key); err != nil { - return err - } - - return nil -} - -func (a *AnonDeviceService) untagUIDevice(ctx context.Context, deviceID string, device *Device) error { - key, err := device.UIKey(deviceID) - if err != nil { - return err - } - - if err := a.remoteCache.Delete(ctx, key); err != nil { - return err - } - - return nil -} - -func (a *AnonDeviceService) tagDeviceUI(ctx context.Context, httpReq *http.Request, device Device) error { - deviceID := httpReq.Header.Get(deviceIDHeader) - if deviceID == "" { - return nil - } - - if device.Kind == anonymous.AnonDevice { - device.Kind = anonymous.AnonDeviceUI - } else if device.Kind == anonymous.AuthedDevice { - device.Kind = anonymous.AuthedDeviceUI - } - - key, err := device.UIKey(deviceID) - if err != nil { - return err - } - - if setting.Env == setting.Dev { - a.log.Debug("Tagging device for UI", "deviceID", deviceID, "device", device, "key", key) - } +func (a *AnonDeviceService) tagDeviceUI(ctx context.Context, httpReq *http.Request, device *anonstore.Device) error { + key := device.CacheKey() if _, ok := a.localCache.Get(key); ok { return nil @@ -147,33 +79,41 @@ func (a *AnonDeviceService) tagDeviceUI(ctx context.Context, httpReq *http.Reque a.localCache.SetDefault(key, struct{}{}) - deviceJSON, err := json.Marshal(device) - if err != nil { - return err + if setting.Env == setting.Dev { + a.log.Debug("Tagging device for UI", "deviceID", device.DeviceID, "device", device, "key", key) } - if err := a.remoteCache.Set(ctx, key, deviceJSON, thirtyDays); err != nil { - return err - } - - // remove existing tag when device switches to another kind - untagKind := anonymous.AnonDeviceUI - if device.Kind == anonymous.AnonDeviceUI { - untagKind = anonymous.AuthedDeviceUI - } - - if err := a.untagUIDevice(ctx, deviceID, &Device{ - Kind: untagKind, - IP: device.IP, - UserAgent: device.UserAgent, - }); err != nil { + if err := a.anonStore.CreateOrUpdateDevice(ctx, device); err != nil { return err } return nil } +func (a *AnonDeviceService) untagDevice(ctx context.Context, + identity *authn.Identity, r *authn.Request, err error) { + if err != nil { + return + } + + deviceID := r.HTTPRequest.Header.Get(deviceIDHeader) + if deviceID == "" { + return + } + + errD := a.anonStore.DeleteDevice(ctx, deviceID) + if errD != nil { + a.log.Debug("Failed to untag device", "error", err) + } +} + +// FIXME: Unexport and remove interface func (a *AnonDeviceService) TagDevice(ctx context.Context, httpReq *http.Request, kind anonymous.DeviceKind) error { + deviceID := httpReq.Header.Get(deviceIDHeader) + if deviceID == "" { + return nil + } + addr := web.RemoteAddr(httpReq) ip, err := network.GetIPFromAddress(addr) if err != nil { @@ -186,54 +126,39 @@ func (a *AnonDeviceService) TagDevice(ctx context.Context, httpReq *http.Request clientIPStr = "" } - taggedDevice := &Device{ - Kind: kind, - IP: clientIPStr, + taggedDevice := &anonstore.Device{ + DeviceID: deviceID, + ClientIP: clientIPStr, UserAgent: httpReq.UserAgent(), - LastSeen: time.Now().UTC(), + CreatedAt: time.Now(), + UpdatedAt: time.Now(), } - err = a.tagDeviceUI(ctx, httpReq, *taggedDevice) + err = a.tagDeviceUI(ctx, httpReq, taggedDevice) if err != nil { a.log.Debug("Failed to tag device for UI", "error", err) } - key, err := taggedDevice.Key() - if err != nil { - return err - } - - if setting.Env == setting.Dev { - a.log.Debug("Tagging device", "device", taggedDevice, "key", key) - } - - if _, ok := a.localCache.Get(key); ok { - return nil - } - - a.localCache.SetDefault(key, struct{}{}) - - deviceJSON, err := json.Marshal(taggedDevice) - if err != nil { - return err - } - - if err := a.remoteCache.Set(ctx, key, deviceJSON, thirtyDays); err != nil { - return err - } - - // remove existing tag when device switches to another kind - untagKind := anonymous.AnonDevice - if kind == anonymous.AnonDevice { - untagKind = anonymous.AuthedDevice - } - if err := a.untagDevice(ctx, &Device{ - Kind: untagKind, - IP: taggedDevice.IP, - UserAgent: taggedDevice.UserAgent, - }); err != nil { - return err - } - return nil } + +func (a *AnonDeviceService) Run(ctx context.Context) error { + ticker := time.NewTicker(2 * time.Hour) + + for { + select { + case <-ticker.C: + err := a.serverLock.LockAndExecute(ctx, "cleanup old anon devices", time.Hour*10, func(context.Context) { + if err := a.anonStore.DeleteDevicesOlderThan(ctx, time.Now().Add(-keepFor)); err != nil { + a.log.Error("An error occurred while deleting old anon devices", "err", err) + } + }) + if err != nil { + a.log.Error("Failed to lock and execute cleanup old anon devices", "error", err) + } + + case <-ctx.Done(): + return ctx.Err() + } + } +} diff --git a/pkg/services/anonymous/anonimpl/impl_test.go b/pkg/services/anonymous/anonimpl/impl_test.go index ec8a0cbd928..a199aa46e9d 100644 --- a/pkg/services/anonymous/anonimpl/impl_test.go +++ b/pkg/services/anonymous/anonimpl/impl_test.go @@ -2,7 +2,6 @@ package anonimpl import ( "context" - "encoding/json" "net/http" "testing" "time" @@ -10,79 +9,30 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/grafana/grafana/pkg/infra/remotecache" + "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/usagestats" "github.com/grafana/grafana/pkg/services/anonymous" + "github.com/grafana/grafana/pkg/services/anonymous/anonimpl/anonstore" + "github.com/grafana/grafana/pkg/services/authn/authntest" + "github.com/grafana/grafana/pkg/services/org/orgtest" + "github.com/grafana/grafana/pkg/setting" ) -func TestAnonDeviceKey(t *testing.T) { - testCases := []struct { - name string - session *Device - expected string - }{ - { - name: "should hash correctly", - session: &Device{ - Kind: anonymous.AnonDevice, - IP: "10.10.10.10", - UserAgent: "test", - }, - expected: "anon-session:ad9f5c6bf504a9fa77c37a3a6658c0cd", - }, - { - name: "should hash correctly with different ip", - session: &Device{ - Kind: anonymous.AnonDevice, - IP: "10.10.10.1", - UserAgent: "test", - }, - expected: "anon-session:580605320245e8289e0b301074a027c3", - }, - { - name: "should hash correctly with different user agent", - session: &Device{ - Kind: anonymous.AnonDevice, - IP: "10.10.10.1", - UserAgent: "test2", - }, - expected: "anon-session:5fdd04b0bd04a9fa77c4243f8111258b", - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - got, err := tc.session.Key() - require.NoError(t, err) - assert.Equal(t, tc.expected, got) - - // ensure that the key is the same - got, err = tc.session.Key() - require.NoError(t, err) - assert.Equal(t, tc.expected, got) - }) - } -} - func TestIntegrationDeviceService_tag(t *testing.T) { type tagReq struct { httpReq *http.Request kind anonymous.DeviceKind } testCases := []struct { - name string - req []tagReq - expectedAnonCount int64 - expectedAuthedCount int64 - expectedAnonUICount int64 - expectedAuthedUICount int64 - expectedDevice *Device + name string + req []tagReq + expectedAnonUICount int64 + expectedKey string + expectedDevice *anonstore.Device }{ { - name: "no requests", - req: []tagReq{{httpReq: &http.Request{}, kind: anonymous.AnonDevice}}, - expectedAnonCount: 0, - expectedAuthedCount: 0, + name: "no requests", + req: []tagReq{{httpReq: &http.Request{}, kind: anonymous.AnonDeviceUI}}, }, { name: "missing info should not tag", @@ -91,28 +41,8 @@ func TestIntegrationDeviceService_tag(t *testing.T) { "User-Agent": []string{"test"}, }, }, - kind: anonymous.AnonDevice, + kind: anonymous.AnonDeviceUI, }}, - expectedAnonCount: 0, - expectedAuthedCount: 0, - }, - { - name: "should tag once", - req: []tagReq{{httpReq: &http.Request{ - Header: http.Header{ - "User-Agent": []string{"test"}, - "X-Forwarded-For": []string{"10.30.30.1"}, - }, - }, - kind: anonymous.AnonDevice, - }, - }, - expectedAnonCount: 1, - expectedAuthedCount: 0, - expectedDevice: &Device{ - Kind: anonymous.AnonDevice, - IP: "10.30.30.1", - UserAgent: "test"}, }, { name: "should tag device ID once", @@ -123,16 +53,14 @@ func TestIntegrationDeviceService_tag(t *testing.T) { http.CanonicalHeaderKey(deviceIDHeader): []string{"32mdo31deeqwes"}, }, }, - kind: anonymous.AnonDevice, + kind: anonymous.AnonDeviceUI, }, }, - expectedAnonUICount: 1, - expectedAuthedUICount: 0, - expectedAnonCount: 1, - expectedAuthedCount: 0, - expectedDevice: &Device{ - Kind: anonymous.AnonDevice, - IP: "10.30.30.1", + expectedAnonUICount: 1, + expectedKey: "ui-anon-session:32mdo31deeqwes", + expectedDevice: &anonstore.Device{ + DeviceID: "32mdo31deeqwes", + ClientIP: "10.30.30.1", UserAgent: "test"}, }, { @@ -144,7 +72,7 @@ func TestIntegrationDeviceService_tag(t *testing.T) { "X-Forwarded-For": []string{"10.30.30.1"}, }, }, - kind: anonymous.AnonDevice, + kind: anonymous.AnonDeviceUI, }, {httpReq: &http.Request{ Header: http.Header{ "User-Agent": []string{"test"}, @@ -152,61 +80,12 @@ func TestIntegrationDeviceService_tag(t *testing.T) { "X-Forwarded-For": []string{"10.30.30.1"}, }, }, - kind: anonymous.AnonDevice, + kind: anonymous.AnonDeviceUI, }, }, - expectedAnonCount: 1, expectedAnonUICount: 1, - expectedAuthedCount: 0, }, { - name: "authed request should untag anon", - req: []tagReq{{httpReq: &http.Request{ - Header: http.Header{ - "User-Agent": []string{"test"}, - http.CanonicalHeaderKey(deviceIDHeader): []string{"32mdo31deeqwes"}, - "X-Forwarded-For": []string{"10.30.30.1"}, - }, - }, - kind: anonymous.AnonDevice, - }, {httpReq: &http.Request{ - Header: http.Header{ - "User-Agent": []string{"test"}, - http.CanonicalHeaderKey(deviceIDHeader): []string{"32mdo31deeqwes"}, - "X-Forwarded-For": []string{"10.30.30.1"}, - }, - }, - kind: anonymous.AuthedDevice, - }, - }, - expectedAnonCount: 0, - expectedAuthedCount: 1, - expectedAuthedUICount: 1, - }, { - name: "anon request should untag authed", - req: []tagReq{{httpReq: &http.Request{ - Header: http.Header{ - "User-Agent": []string{"test"}, - http.CanonicalHeaderKey(deviceIDHeader): []string{"32mdo31deeqwes"}, - "X-Forwarded-For": []string{"10.30.30.1"}, - }, - }, - kind: anonymous.AuthedDevice, - }, {httpReq: &http.Request{ - Header: http.Header{ - "User-Agent": []string{"test"}, - http.CanonicalHeaderKey(deviceIDHeader): []string{"32mdo31deeqwes"}, - "X-Forwarded-For": []string{"10.30.30.1"}, - }, - }, - kind: anonymous.AnonDevice, - }, - }, - expectedAnonCount: 1, - expectedAnonUICount: 1, - expectedAuthedCount: 0, - }, - { - name: "tag 4 different requests - 2 are UI", + name: "tag 2 different requests", req: []tagReq{{httpReq: &http.Request{ Header: http.Header{ http.CanonicalHeaderKey("User-Agent"): []string{"test"}, @@ -214,106 +93,87 @@ func TestIntegrationDeviceService_tag(t *testing.T) { http.CanonicalHeaderKey(deviceIDHeader): []string{"a"}, }, }, - kind: anonymous.AnonDevice, - }, {httpReq: &http.Request{ - Header: http.Header{ - "User-Agent": []string{"test"}, - "X-Forwarded-For": []string{"10.30.30.2"}, - }, - }, - kind: anonymous.AnonDevice, + kind: anonymous.AnonDeviceUI, }, {httpReq: &http.Request{ Header: http.Header{ "User-Agent": []string{"test"}, - "X-Forwarded-For": []string{"10.30.30.3"}, - http.CanonicalHeaderKey(deviceIDHeader): []string{"c"}, + "X-Forwarded-For": []string{"10.30.30.2"}, + http.CanonicalHeaderKey(deviceIDHeader): []string{"b"}, }, }, - kind: anonymous.AuthedDevice, - }, {httpReq: &http.Request{ - Header: http.Header{ - "User-Agent": []string{"test"}, - "X-Forwarded-For": []string{"10.30.30.4"}, - }, - }, - kind: anonymous.AuthedDevice, + kind: anonymous.AnonDeviceUI, }, }, - expectedAnonCount: 2, - expectedAuthedCount: 2, - expectedAnonUICount: 1, - expectedAuthedUICount: 1, + expectedAnonUICount: 2, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - fakeStore := remotecache.NewFakeStore(t) - - anonService := ProvideAnonymousDeviceService(fakeStore, &usagestats.UsageStatsMock{}) + store := db.InitTestDB(t) + anonDBStore := anonstore.ProvideAnonDBStore(store) + anonService := ProvideAnonymousDeviceService(&usagestats.UsageStatsMock{}, + &authntest.FakeService{}, anonDBStore, setting.NewCfg(), orgtest.NewOrgServiceFake(), nil) 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) + require.NoError(t, err) + require.Len(t, devices, int(tc.expectedAnonUICount)) + if tc.expectedDevice != nil { + device := devices[0] + assert.NotZero(t, device.ID) + assert.NotZero(t, device.CreatedAt) + assert.NotZero(t, device.UpdatedAt) + + tc.expectedDevice.ID = device.ID + tc.expectedDevice.CreatedAt = device.CreatedAt + tc.expectedDevice.UpdatedAt = device.UpdatedAt + + assert.Equal(t, tc.expectedDevice, devices[0]) + } + stats, err := anonService.usageStatFn(context.Background()) require.NoError(t, err) - assert.Equal(t, tc.expectedAnonCount, stats["stats.anonymous.session.count"].(int64)) - assert.Equal(t, tc.expectedAuthedCount, stats["stats.users.device.count"].(int64)) - assert.Equal(t, tc.expectedAnonUICount, stats["stats.anonymous.device.ui.count"].(int64)) - assert.Equal(t, tc.expectedAuthedUICount, stats["stats.users.device.ui.count"].(int64)) - - if tc.expectedDevice != nil { - key, err := tc.expectedDevice.Key() - require.NoError(t, err) - - k, err := fakeStore.Get(context.Background(), key) - require.NoError(t, err) - - gotDevice := &Device{} - err = json.Unmarshal(k, gotDevice) - require.NoError(t, err) - - assert.NotNil(t, gotDevice.LastSeen) - gotDevice.LastSeen = time.Time{} - - assert.Equal(t, tc.expectedDevice, gotDevice) - } + assert.Equal(t, tc.expectedAnonUICount, stats["stats.anonymous.device.ui.count"].(int64), stats) }) } } // Ensure that the local cache prevents request from being tagged func TestIntegrationAnonDeviceService_localCacheSafety(t *testing.T) { - fakeStore := remotecache.NewFakeStore(t) - anonService := ProvideAnonymousDeviceService(fakeStore, &usagestats.UsageStatsMock{}) + store := db.InitTestDB(t) + anonDBStore := anonstore.ProvideAnonDBStore(store) + anonService := ProvideAnonymousDeviceService(&usagestats.UsageStatsMock{}, + &authntest.FakeService{}, anonDBStore, setting.NewCfg(), orgtest.NewOrgServiceFake(), nil) req := &http.Request{ Header: http.Header{ - "User-Agent": []string{"test"}, - "X-Forwarded-For": []string{"10.30.30.2"}, + "User-Agent": []string{"test"}, + "X-Forwarded-For": []string{"10.30.30.2"}, + http.CanonicalHeaderKey(deviceIDHeader): []string{"32mdo31deeqwes"}, }, } - anonDevice := &Device{ - Kind: anonymous.AnonDevice, - IP: "10.30.30.2", + anonDevice := &anonstore.Device{ + DeviceID: "32mdo31deeqwes", + ClientIP: "10.30.30.2", UserAgent: "test", - LastSeen: time.Now().UTC(), + UpdatedAt: time.Now().UTC(), } - key, err := anonDevice.Key() - require.NoError(t, err) - + key := anonDevice.CacheKey() anonService.localCache.SetDefault(key, true) - err = anonService.TagDevice(context.Background(), req, anonymous.AnonDevice) + err := anonService.TagDevice(context.Background(), req, anonymous.AnonDeviceUI) require.NoError(t, err) stats, err := anonService.usageStatFn(context.Background()) require.NoError(t, err) - assert.Equal(t, int64(0), stats["stats.anonymous.session.count"].(int64)) + assert.Equal(t, int64(0), stats["stats.anonymous.device.ui.count"].(int64)) } diff --git a/pkg/services/anonymous/service.go b/pkg/services/anonymous/service.go index 8a60c240bae..b303d705843 100644 --- a/pkg/services/anonymous/service.go +++ b/pkg/services/anonymous/service.go @@ -8,10 +8,7 @@ import ( type DeviceKind string const ( - AnonDevice DeviceKind = "anon-session" - AuthedDevice DeviceKind = "authed-session" - AnonDeviceUI DeviceKind = "ui-anon-session" - AuthedDeviceUI DeviceKind = "ui-authed-session" + AnonDeviceUI DeviceKind = "ui-anon-session" ) type Service interface { diff --git a/pkg/services/authn/authnimpl/service.go b/pkg/services/authn/authnimpl/service.go index 29a07ae54a2..432b0e53648 100644 --- a/pkg/services/authn/authnimpl/service.go +++ b/pkg/services/authn/authnimpl/service.go @@ -16,7 +16,6 @@ import ( "github.com/grafana/grafana/pkg/infra/usagestats" "github.com/grafana/grafana/pkg/login/social" "github.com/grafana/grafana/pkg/services/accesscontrol" - "github.com/grafana/grafana/pkg/services/anonymous" "github.com/grafana/grafana/pkg/services/apikey" "github.com/grafana/grafana/pkg/services/auth" "github.com/grafana/grafana/pkg/services/authn" @@ -64,7 +63,6 @@ func ProvideService( apikeyService apikey.Service, userService user.Service, jwtService auth.JWTVerifierService, usageStats usagestats.Service, - anonDeviceService anonymous.Service, userProtectionService login.UserProtectionService, loginAttempts loginattempt.Service, quotaService quota.Service, authInfoService login.AuthInfoService, renderService rendering.Service, @@ -91,11 +89,7 @@ func ProvideService( s.RegisterClient(clients.ProvideAPIKey(apikeyService, userService)) if cfg.LoginCookieName != "" { - s.RegisterClient(clients.ProvideSession(cfg, sessionService, features, anonDeviceService)) - } - - if s.cfg.AnonymousEnabled { - s.RegisterClient(clients.ProvideAnonymous(cfg, orgService, anonDeviceService)) + s.RegisterClient(clients.ProvideSession(cfg, sessionService, features)) } var proxyClients []authn.ProxyClient diff --git a/pkg/services/authn/clients/session.go b/pkg/services/authn/clients/session.go index 9e65e86f2c0..c97a87d27f1 100644 --- a/pkg/services/authn/clients/session.go +++ b/pkg/services/authn/clients/session.go @@ -3,13 +3,11 @@ package clients import ( "context" "errors" - "net/http" "net/url" "time" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/network" - "github.com/grafana/grafana/pkg/services/anonymous" "github.com/grafana/grafana/pkg/services/auth" "github.com/grafana/grafana/pkg/services/authn" "github.com/grafana/grafana/pkg/services/featuremgmt" @@ -21,24 +19,20 @@ var _ authn.HookClient = new(Session) var _ authn.ContextAwareClient = new(Session) func ProvideSession(cfg *setting.Cfg, sessionService auth.UserTokenService, - features *featuremgmt.FeatureManager, anonDeviceService anonymous.Service) *Session { + features *featuremgmt.FeatureManager) *Session { return &Session{ - cfg: cfg, - features: features, - sessionService: sessionService, - log: log.New(authn.ClientSession), - anonDeviceService: anonDeviceService, - tagDevices: cfg.TagAuthedDevices, + cfg: cfg, + features: features, + sessionService: sessionService, + log: log.New(authn.ClientSession), } } type Session struct { - cfg *setting.Cfg - features *featuremgmt.FeatureManager - sessionService auth.UserTokenService - log log.Logger - tagDevices bool - anonDeviceService anonymous.Service + cfg *setting.Cfg + features *featuremgmt.FeatureManager + sessionService auth.UserTokenService + log log.Logger } func (s *Session) Name() string { @@ -67,29 +61,6 @@ func (s *Session) Authenticate(ctx context.Context, r *authn.Request) (*authn.Id } } - if s.tagDevices { - // Tag authed devices - httpReqCopy := &http.Request{} - if r.HTTPRequest != nil && r.HTTPRequest.Header != nil { - // avoid r.HTTPRequest.Clone(context.Background()) as we do not require a full clone - httpReqCopy.Header = r.HTTPRequest.Header.Clone() - httpReqCopy.RemoteAddr = r.HTTPRequest.RemoteAddr - } - go func() { - defer func() { - if err := recover(); err != nil { - s.log.Warn("Tag anon session panic", "err", err) - } - }() - - newCtx, cancel := context.WithTimeout(context.Background(), timeoutTag) - defer cancel() - if err := s.anonDeviceService.TagDevice(newCtx, httpReqCopy, anonymous.AuthedDevice); err != nil { - s.log.Warn("Failed to tag anonymous session", "error", err) - } - }() - } - return &authn.Identity{ ID: authn.NamespacedID(authn.NamespaceUser, token.UserId), SessionToken: token, diff --git a/pkg/services/authn/clients/session_test.go b/pkg/services/authn/clients/session_test.go index c394b6735d5..8dd7555cd26 100644 --- a/pkg/services/authn/clients/session_test.go +++ b/pkg/services/authn/clients/session_test.go @@ -11,7 +11,6 @@ import ( "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/models/usertoken" - "github.com/grafana/grafana/pkg/services/anonymous/anontest" "github.com/grafana/grafana/pkg/services/auth" "github.com/grafana/grafana/pkg/services/auth/authtest" "github.com/grafana/grafana/pkg/services/authn" @@ -30,7 +29,7 @@ func TestSession_Test(t *testing.T) { cfg := setting.NewCfg() cfg.LoginCookieName = "" cfg.LoginMaxLifetime = 20 * time.Second - s := ProvideSession(cfg, &authtest.FakeUserAuthTokenService{}, featuremgmt.WithFeatures(), &anontest.FakeAnonymousSessionService{}) + s := ProvideSession(cfg, &authtest.FakeUserAuthTokenService{}, featuremgmt.WithFeatures()) disabled := s.Test(context.Background(), &authn.Request{HTTPRequest: validHTTPReq}) assert.False(t, disabled) @@ -146,7 +145,7 @@ func TestSession_Authenticate(t *testing.T) { cfg.LoginCookieName = cookieName cfg.TokenRotationIntervalMinutes = 10 cfg.LoginMaxLifetime = 20 * time.Second - s := ProvideSession(cfg, tt.fields.sessionService, tt.fields.features, &anontest.FakeAnonymousSessionService{}) + s := ProvideSession(cfg, tt.fields.sessionService, tt.fields.features) got, err := s.Authenticate(context.Background(), tt.args.r) require.True(t, (err != nil) == tt.wantErr, err) @@ -186,7 +185,7 @@ func TestSession_Hook(t *testing.T) { token.UnhashedToken = "new-token" return true, token, nil }, - }, featuremgmt.WithFeatures(), &anontest.FakeAnonymousSessionService{}) + }, featuremgmt.WithFeatures()) sampleID := &authn.Identity{ SessionToken: &auth.UserToken{ @@ -220,7 +219,7 @@ func TestSession_Hook(t *testing.T) { }) t.Run("should not rotate token with feature flag", func(t *testing.T) { - s := ProvideSession(setting.NewCfg(), nil, featuremgmt.WithFeatures(featuremgmt.FlagClientTokenRotation), &anontest.FakeAnonymousSessionService{}) + s := ProvideSession(setting.NewCfg(), nil, featuremgmt.WithFeatures(featuremgmt.FlagClientTokenRotation)) req := &authn.Request{} identity := &authn.Identity{} diff --git a/pkg/services/sqlstore/migrations/anonservice/migrations.go b/pkg/services/sqlstore/migrations/anonservice/migrations.go new file mode 100644 index 00000000000..8d7469d43b4 --- /dev/null +++ b/pkg/services/sqlstore/migrations/anonservice/migrations.go @@ -0,0 +1,25 @@ +package anonservice + +import "github.com/grafana/grafana/pkg/services/sqlstore/migrator" + +func AddMigration(mg *migrator.Migrator) { + var anonV1 = migrator.Table{ + Name: "anon_device", + Columns: []*migrator.Column{ + {Name: "id", Type: migrator.DB_BigInt, IsPrimaryKey: true, IsAutoIncrement: true}, + {Name: "client_ip", Type: migrator.DB_NVarchar, Length: 255, Nullable: false}, + {Name: "created_at", Type: migrator.DB_DateTime, Nullable: false}, + {Name: "device_id", Type: migrator.DB_NVarchar, Length: 127, Nullable: false}, + {Name: "updated_at", Type: migrator.DB_DateTime, Nullable: false}, + {Name: "user_agent", Type: migrator.DB_NVarchar, Length: 255, Nullable: false}, + }, + Indices: []*migrator.Index{ + {Cols: []string{"device_id"}, Type: migrator.UniqueIndex}, + {Cols: []string{"updated_at"}, Type: migrator.IndexType}, + }, + } + + mg.AddMigration("create anon_device table", migrator.NewAddTableMigration(anonV1)) + mg.AddMigration("add unique index anon_device.device_id", migrator.NewAddIndexMigration(anonV1, anonV1.Indices[0])) + mg.AddMigration("add index anon_device.updated_at", migrator.NewAddIndexMigration(anonV1, anonV1.Indices[1])) +} diff --git a/pkg/services/sqlstore/migrations/migrations.go b/pkg/services/sqlstore/migrations/migrations.go index 75867ea2f1d..dd98d9822fc 100644 --- a/pkg/services/sqlstore/migrations/migrations.go +++ b/pkg/services/sqlstore/migrations/migrations.go @@ -3,6 +3,7 @@ package migrations import ( "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/sqlstore/migrations/accesscontrol" + "github.com/grafana/grafana/pkg/services/sqlstore/migrations/anonservice" "github.com/grafana/grafana/pkg/services/sqlstore/migrations/oauthserver" "github.com/grafana/grafana/pkg/services/sqlstore/migrations/ualert" . "github.com/grafana/grafana/pkg/services/sqlstore/migrator" @@ -96,6 +97,8 @@ func (*OSSMigrations) AddMigration(mg *Migrator) { oauthserver.AddMigration(mg) } } + + anonservice.AddMigration(mg) } func addStarMigrations(mg *Migrator) { diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index f7354c4562a..42260e9eae7 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -287,8 +287,6 @@ type Cfg struct { // Not documented & not supported // stand in until a more complete solution is implemented AuthConfigUIAdminAccess bool - // TO REMOVE: Not documented & not supported. Remove in 10.3 - TagAuthedDevices bool // AWS Plugin Auth AWSAllowedAuthProviders []string @@ -1551,7 +1549,6 @@ func readAuthSettings(iniFile *ini.File, cfg *Cfg) (err error) { // Do not use cfg.AuthConfigUIAdminAccess = auth.Key("config_ui_admin_access").MustBool(false) - cfg.TagAuthedDevices = auth.Key("tag_authed_devices").MustBool(true) cfg.DisableLoginForm = auth.Key("disable_login_form").MustBool(false) DisableSignoutMenu = auth.Key("disable_signout_menu").MustBool(false)