diff --git a/pkg/api/user_test.go b/pkg/api/user_test.go index 57284287f60..16c9fcfaf51 100644 --- a/pkg/api/user_test.go +++ b/pkg/api/user_test.go @@ -18,7 +18,6 @@ import ( "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/db/dbtest" - "github.com/grafana/grafana/pkg/infra/remotecache" "github.com/grafana/grafana/pkg/login/social" "github.com/grafana/grafana/pkg/login/social/socialtest" "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" @@ -65,7 +64,8 @@ func TestUserAPIEndpoint_userLoggedIn(t *testing.T) { secretsService := secretsManager.SetupTestService(t, database.ProvideSecretsStore(sqlStore)) authInfoStore := authinfoimpl.ProvideStore(sqlStore, secretsService) srv := authinfoimpl.ProvideService( - authInfoStore, remotecache.NewFakeCacheStorage(), secretsService) + authInfoStore, + ) hs.authInfoService = srv orgSvc, err := orgimpl.ProvideService(sqlStore, sqlStore.Cfg, quotatest.New(false, nil)) require.NoError(t, err) diff --git a/pkg/infra/remotecache/redis_storage.go b/pkg/infra/remotecache/redis_storage.go index e97679c04f6..13684530525 100644 --- a/pkg/infra/remotecache/redis_storage.go +++ b/pkg/infra/remotecache/redis_storage.go @@ -3,7 +3,6 @@ package remotecache import ( "context" "crypto/tls" - "errors" "fmt" "strconv" "strings" @@ -94,15 +93,7 @@ func (s *redisStorage) Set(ctx context.Context, key string, data []byte, expires // GetByteArray returns the value as byte array func (s *redisStorage) Get(ctx context.Context, key string) ([]byte, error) { - item, err := s.c.Get(ctx, key).Bytes() - if err != nil { - if errors.Is(err, redis.Nil) { - return nil, ErrCacheItemNotFound - } - return nil, err - } - - return item, nil + return s.c.Get(ctx, key).Bytes() } // Delete delete a key from session. diff --git a/pkg/services/login/authinfoimpl/service.go b/pkg/services/login/authinfoimpl/service.go index c6e87a6bbbe..bb4759febe0 100644 --- a/pkg/services/login/authinfoimpl/service.go +++ b/pkg/services/login/authinfoimpl/service.go @@ -2,66 +2,27 @@ package authinfoimpl import ( "context" - "encoding/json" - "errors" - "strconv" - "time" "github.com/grafana/grafana/pkg/infra/log" - "github.com/grafana/grafana/pkg/infra/remotecache" "github.com/grafana/grafana/pkg/services/login" - "github.com/grafana/grafana/pkg/services/secrets" - "github.com/grafana/grafana/pkg/services/user" ) type Service struct { authInfoStore login.Store logger log.Logger - remoteCache remotecache.CacheStorage - secretService secrets.Service } -const remoteCachePrefix = "authinfo-" -const remoteCacheTTL = 60 * time.Hour - -func ProvideService(authInfoStore login.Store, - remoteCache remotecache.CacheStorage, - secretService secrets.Service) *Service { +func ProvideService(authInfoStore login.Store) *Service { s := &Service{ authInfoStore: authInfoStore, logger: log.New("login.authinfo"), - remoteCache: remoteCache, - secretService: secretService, } return s } func (s *Service) GetAuthInfo(ctx context.Context, query *login.GetAuthInfoQuery) (*login.UserAuth, error) { - if query.UserId == 0 && query.AuthId == "" { - return nil, user.ErrUserNotFound - } - - authInfo, err := s.getAuthInfoFromCache(ctx, query) - if err != nil && !errors.Is(err, remotecache.ErrCacheItemNotFound) { - s.logger.Error("failed to retrieve auth info from cache", "error", err) - } else if authInfo != nil { - return authInfo, nil - } - - authInfo, err = s.authInfoStore.GetAuthInfo(ctx, query) - if err != nil { - return nil, err - } - - err = s.setAuthInfoInCache(ctx, query, authInfo) - if err != nil { - s.logger.Error("failed to set auth info in cache", "error", err) - } else { - s.logger.Debug("auth info set in cache", "cacheKey", generateCacheKey(query)) - } - - return authInfo, nil + return s.authInfoStore.GetAuthInfo(ctx, query) } func (s *Service) GetUserLabels(ctx context.Context, query login.GetUserLabelsQuery) (map[int64]string, error) { @@ -71,98 +32,14 @@ func (s *Service) GetUserLabels(ctx context.Context, query login.GetUserLabelsQu return s.authInfoStore.GetUserLabels(ctx, query) } -func (s *Service) setAuthInfoInCache(ctx context.Context, query *login.GetAuthInfoQuery, info *login.UserAuth) error { - cacheKey := generateCacheKey(query) - infoJSON, err := json.Marshal(info) - if err != nil { - return err - } - - encryptedInfo, err := s.secretService.Encrypt(ctx, infoJSON, secrets.WithoutScope()) - if err != nil { - return err - } - - return s.remoteCache.Set(ctx, cacheKey, encryptedInfo, remoteCacheTTL) -} - -func (s *Service) getAuthInfoFromCache(ctx context.Context, query *login.GetAuthInfoQuery) (*login.UserAuth, error) { - // check if we have the auth info in the remote cache - cacheKey := generateCacheKey(query) - item, err := s.remoteCache.Get(ctx, cacheKey) - if err != nil { - return nil, err - } - - info := &login.UserAuth{} - itemJSON, err := s.secretService.Decrypt(ctx, item) - if err != nil { - return nil, err - } - - if err := json.Unmarshal(itemJSON, info); err != nil { - return nil, err - } - - s.logger.Debug("auth info retrieved from cache", "cacheKey", cacheKey) - - return info, nil -} - -func generateCacheKey(query *login.GetAuthInfoQuery) string { - cacheKey := remoteCachePrefix + strconv.FormatInt(query.UserId, 10) + "-" + - query.AuthModule + "-" + query.AuthId - return cacheKey -} - func (s *Service) UpdateAuthInfo(ctx context.Context, cmd *login.UpdateAuthInfoCommand) error { - err := s.authInfoStore.UpdateAuthInfo(ctx, cmd) - if err != nil { - return err - } - - err = s.remoteCache.Delete(ctx, generateCacheKey(&login.GetAuthInfoQuery{ - UserId: cmd.UserId, - AuthModule: cmd.AuthModule, - AuthId: cmd.AuthId, - })) - if err != nil { - s.logger.Error("failed to delete auth info from cache", "error", err) - } - - return nil + return s.authInfoStore.UpdateAuthInfo(ctx, cmd) } func (s *Service) SetAuthInfo(ctx context.Context, cmd *login.SetAuthInfoCommand) error { - err := s.authInfoStore.SetAuthInfo(ctx, cmd) - if err != nil { - return err - } - - err = s.remoteCache.Delete(ctx, generateCacheKey(&login.GetAuthInfoQuery{ - UserId: cmd.UserId, - AuthModule: cmd.AuthModule, - AuthId: cmd.AuthId, - })) - if err != nil { - s.logger.Error("failed to delete auth info from cache", "error", err) - } - - return nil + return s.authInfoStore.SetAuthInfo(ctx, cmd) } func (s *Service) DeleteUserAuthInfo(ctx context.Context, userID int64) error { - err := s.authInfoStore.DeleteUserAuthInfo(ctx, userID) - if err != nil { - return err - } - - err = s.remoteCache.Delete(ctx, generateCacheKey(&login.GetAuthInfoQuery{ - UserId: userID, - })) - if err != nil { - s.logger.Error("failed to delete auth info from cache", "error", err) - } - - return nil + return s.authInfoStore.DeleteUserAuthInfo(ctx, userID) } diff --git a/pkg/services/oauthtoken/oauth_token_test.go b/pkg/services/oauthtoken/oauth_token_test.go index c010084e022..28e56398ac6 100644 --- a/pkg/services/oauthtoken/oauth_token_test.go +++ b/pkg/services/oauthtoken/oauth_token_test.go @@ -10,16 +10,12 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/require" "golang.org/x/oauth2" "golang.org/x/sync/singleflight" - "github.com/grafana/grafana/pkg/infra/remotecache" "github.com/grafana/grafana/pkg/login/social/socialtest" "github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/login/authinfoimpl" - "github.com/grafana/grafana/pkg/services/secrets/fakes" - secretsManager "github.com/grafana/grafana/pkg/services/secrets/manager" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" ) @@ -121,9 +117,10 @@ func TestService_TryTokenRefresh_ValidToken(t *testing.T) { assert.Nil(t, err) socialConnector.AssertNumberOfCalls(t, "TokenSource", 1) - authInfoQuery := &login.GetAuthInfoQuery{UserId: 1} + authInfoQuery := &login.GetAuthInfoQuery{} resultUsr, err := srv.AuthInfoService.GetAuthInfo(ctx, authInfoQuery) - require.Nil(t, err) + + assert.Nil(t, err) // User's token data had not been updated assert.Equal(t, resultUsr.OAuthAccessToken, token.AccessToken) @@ -193,10 +190,10 @@ func TestService_TryTokenRefresh_ExpiredToken(t *testing.T) { assert.Nil(t, err) socialConnector.AssertNumberOfCalls(t, "TokenSource", 1) - authInfoQuery := &login.GetAuthInfoQuery{UserId: 1} + authInfoQuery := &login.GetAuthInfoQuery{} authInfo, err := srv.AuthInfoService.GetAuthInfo(ctx, authInfoQuery) - require.Nil(t, err) + assert.Nil(t, err) // newToken should be returned after the .Token() call, therefore the User had to be updated assert.Equal(t, authInfo.OAuthAccessToken, newToken.AccessToken) @@ -232,8 +229,7 @@ func setupOAuthTokenService(t *testing.T) (*Service, *FakeAuthInfoStore, *social } authInfoStore := &FakeAuthInfoStore{} - authInfoService := authinfoimpl.ProvideService(authInfoStore, remotecache.NewFakeCacheStorage(), - secretsManager.SetupTestService(t, fakes.NewFakeSecretsStore())) + authInfoService := authinfoimpl.ProvideService(authInfoStore) return &Service{ Cfg: setting.NewCfg(), SocialService: socialService,