From 64c2032c2bde7f92b7fd48873cbe95ede6f28501 Mon Sep 17 00:00:00 2001 From: Carl Bergquist Date: Mon, 6 Feb 2023 13:51:12 +0100 Subject: [PATCH] Auth: removes temporary cache of user session token (#62730) Signed-off-by: bergquist --- .../feature-toggles/index.md | 1 - .../src/types/featureToggles.gen.ts | 1 - pkg/services/auth/authimpl/auth_token.go | 61 +------------------ pkg/services/featuremgmt/registry.go | 5 -- pkg/services/featuremgmt/toggles_gen.go | 4 -- pkg/services/quota/quotaimpl/quota_test.go | 2 +- 6 files changed, 2 insertions(+), 72 deletions(-) diff --git a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md index 5cc985a2316..a2deca3e512 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -91,7 +91,6 @@ Alpha features might be changed or removed without prior notice. | `datasourceOnboarding` | Enable data source onboarding page | | `secureSocksDatasourceProxy` | Enable secure socks tunneling for supported core datasources | | `authnService` | Use new auth service to perform authentication | -| `sessionRemoteCache` | Enable using remote cache for user sessions | | `alertingBacktesting` | Rule backtesting API for alerting | | `editPanelCSVDragAndDrop` | Enables drag and drop for CSV and Excel files | | `logsContextDatasourceUi` | Allow datasource to provide custom UI for context view | diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index 37a74f86dc6..58cfcf7d887 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -80,7 +80,6 @@ export interface FeatureToggles { datasourceOnboarding?: boolean; secureSocksDatasourceProxy?: boolean; authnService?: boolean; - sessionRemoteCache?: boolean; disablePrometheusExemplarSampling?: boolean; alertingBacktesting?: boolean; editPanelCSVDragAndDrop?: boolean; diff --git a/pkg/services/auth/authimpl/auth_token.go b/pkg/services/auth/authimpl/auth_token.go index 4f9061d4c79..b1fee6be35a 100644 --- a/pkg/services/auth/authimpl/auth_token.go +++ b/pkg/services/auth/authimpl/auth_token.go @@ -4,7 +4,6 @@ import ( "context" "crypto/sha256" "encoding/hex" - "errors" "fmt" "net" "strings" @@ -14,27 +13,20 @@ import ( "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/log" - "github.com/grafana/grafana/pkg/infra/remotecache" "github.com/grafana/grafana/pkg/infra/serverlock" "github.com/grafana/grafana/pkg/services/auth" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/quota" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" ) -const ( - ttl = 15 * time.Second - urgentRotateTime = 1 * time.Minute -) +const urgentRotateTime = 1 * time.Minute var getTime = time.Now func ProvideUserAuthTokenService(sqlStore db.DB, serverLockService *serverlock.ServerLockService, - remoteCache *remotecache.RemoteCache, - features *featuremgmt.FeatureManager, quotaService quota.Service, cfg *setting.Cfg) (*UserAuthTokenService, error) { s := &UserAuthTokenService{ @@ -42,8 +34,6 @@ func ProvideUserAuthTokenService(sqlStore db.DB, serverLockService: serverLockService, cfg: cfg, log: log.New("auth"), - remoteCache: remoteCache, - features: features, singleflight: new(singleflight.Group), } @@ -60,8 +50,6 @@ func ProvideUserAuthTokenService(sqlStore db.DB, return s, err } - remotecache.Register(auth.UserToken{}) - return s, nil } @@ -70,8 +58,6 @@ type UserAuthTokenService struct { serverLockService *serverlock.ServerLockService cfg *setting.Cfg log log.Logger - remoteCache *remotecache.RemoteCache - features *featuremgmt.FeatureManager singleflight *singleflight.Group } @@ -123,52 +109,7 @@ func (s *UserAuthTokenService) CreateToken(ctx context.Context, user *user.User, return &userToken, err } -func (s *UserAuthTokenService) lookupTokenWithCache(ctx context.Context, unhashedToken string) (*auth.UserToken, error) { - hashedToken := hashToken(unhashedToken) - cacheKey := "auth_token:" + hashedToken - - session, errCache := s.remoteCache.Get(ctx, cacheKey) - if errCache == nil { - token := session.(auth.UserToken) - return &token, nil - } else { - if errors.Is(errCache, remotecache.ErrCacheItemNotFound) { - s.log.Debug("user auth token not found in cache", - "cacheKey", cacheKey) - } else { - s.log.Warn("failed to get user auth token from cache", - "cacheKey", cacheKey, "error", errCache) - } - } - - token, err := s.lookupToken(ctx, unhashedToken) - if err != nil { - return nil, err - } - - // only cache tokens until their near rotation time - // Near rotation time = tokens last rotation plus the rotation interval minus 2 ttl (=30s by default) - nextRotation := time.Unix(token.RotatedAt, 0). - Add(-2 * ttl). // subtract 2 ttl to make sure we don't cache tokens that are about to expire - Add(time.Duration(s.cfg.TokenRotationIntervalMinutes) * time.Minute) - if now := getTime(); now.Before(nextRotation) { - if err := s.remoteCache.Set(ctx, cacheKey, *token, ttl); err != nil { - s.log.Warn("could not cache token", "error", err, "cacheKey", cacheKey, "userId", token.UserId) - } - } - - return token, nil -} - func (s *UserAuthTokenService) LookupToken(ctx context.Context, unhashedToken string) (*auth.UserToken, error) { - if s.features != nil && s.features.IsEnabled(featuremgmt.FlagSessionRemoteCache) { - return s.lookupTokenWithCache(ctx, unhashedToken) - } - - return s.lookupToken(ctx, unhashedToken) -} - -func (s *UserAuthTokenService) lookupToken(ctx context.Context, unhashedToken string) (*auth.UserToken, error) { hashedToken := hashToken(unhashedToken) var model userAuthToken var exists bool diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index 55fb52af7c0..f5f6b26f871 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -357,11 +357,6 @@ var ( Description: "Use new auth service to perform authentication", State: FeatureStateAlpha, }, - { - Name: "sessionRemoteCache", - Description: "Enable using remote cache for user sessions", - State: FeatureStateAlpha, - }, { Name: "disablePrometheusExemplarSampling", Description: "Disable Prometheus examplar sampling", diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index 9842d73cd1f..c4155aa6227 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -263,10 +263,6 @@ const ( // Use new auth service to perform authentication FlagAuthnService = "authnService" - // FlagSessionRemoteCache - // Enable using remote cache for user sessions - FlagSessionRemoteCache = "sessionRemoteCache" - // FlagDisablePrometheusExemplarSampling // Disable Prometheus examplar sampling FlagDisablePrometheusExemplarSampling = "disablePrometheusExemplarSampling" diff --git a/pkg/services/quota/quotaimpl/quota_test.go b/pkg/services/quota/quotaimpl/quota_test.go index 0a622b678f4..d98632e72de 100644 --- a/pkg/services/quota/quotaimpl/quota_test.go +++ b/pkg/services/quota/quotaimpl/quota_test.go @@ -467,7 +467,7 @@ func setupEnv(t *testing.T, sqlStore *sqlstore.SQLStore, b bus.Bus, quotaService tracer := tracing.InitializeTracerForTest() _, err := apikeyimpl.ProvideService(sqlStore, sqlStore.Cfg, quotaService) require.NoError(t, err) - _, err = authimpl.ProvideUserAuthTokenService(sqlStore, nil, nil, featuremgmt.WithFeatures(), quotaService, sqlStore.Cfg) + _, err = authimpl.ProvideUserAuthTokenService(sqlStore, nil, quotaService, sqlStore.Cfg) require.NoError(t, err) _, err = dashboardStore.ProvideDashboardStore(sqlStore, sqlStore.Cfg, featuremgmt.WithFeatures(), tagimpl.ProvideService(sqlStore, sqlStore.Cfg), quotaService) require.NoError(t, err)