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 30f711ef6ad..5e61547c94b 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -35,7 +35,6 @@ Some features are enabled by default. You can disable these feature by setting t | `lokiQuerySplitting` | Split large interval queries into subqueries with smaller time intervals | Yes | | `prometheusMetricEncyclopedia` | Adds the metrics explorer component to the Prometheus query builder as an option in metric select | Yes | | `influxdbBackendMigration` | Query InfluxDB InfluxQL without the proxy | Yes | -| `clientTokenRotation` | Replaces the current in-request token rotation so that the client initiates the rotation | Yes | | `prometheusDataplane` | Changes responses to from Prometheus to be compliant with the dataplane specification. In particular, when this feature toggle is active, the numeric `Field.Name` is set from 'Value' to the value of the `__name__` label. | Yes | | `lokiMetricDataplane` | Changes metric responses from Loki to be compliant with the dataplane specification. | Yes | | `dataplaneFrontendFallback` | Support dataplane contract field name change for transformations and field name matchers where the name is different | Yes | diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index 1075f27a677..428f74ad718 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -68,7 +68,6 @@ export interface FeatureToggles { influxdbBackendMigration?: boolean; influxqlStreamingParser?: boolean; influxdbRunQueriesInParallel?: boolean; - clientTokenRotation?: boolean; prometheusDataplane?: boolean; lokiMetricDataplane?: boolean; lokiLogsDataplane?: boolean; diff --git a/pkg/api/api.go b/pkg/api/api.go index fe5bfc191dd..ec95b4a7b2f 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -213,10 +213,8 @@ func (hs *HTTPServer) registerRoutes() { // add swagger support registerSwaggerUI(r) - if hs.Features.IsEnabledGlobally(featuremgmt.FlagClientTokenRotation) { - r.Post("/api/user/auth-tokens/rotate", routing.Wrap(hs.RotateUserAuthToken)) - r.Get("/user/auth-tokens/rotate", routing.Wrap(hs.RotateUserAuthTokenRedirect)) - } + r.Post("/api/user/auth-tokens/rotate", routing.Wrap(hs.RotateUserAuthToken)) + r.Get("/user/auth-tokens/rotate", routing.Wrap(hs.RotateUserAuthTokenRedirect)) adminAuthPageEvaluator := func() ac.Evaluator { authnSettingsEval := ssoutils.EvalAuthenticationSettings(hs.Cfg) diff --git a/pkg/api/login.go b/pkg/api/login.go index 9f6376a5469..90b2d433a4a 100644 --- a/pkg/api/login.go +++ b/pkg/api/login.go @@ -89,11 +89,9 @@ func (hs *HTTPServer) CookieOptionsFromCfg() cookies.CookieOptions { } func (hs *HTTPServer) LoginView(c *contextmodel.ReqContext) { - if hs.Features.IsEnabled(c.Req.Context(), featuremgmt.FlagClientTokenRotation) { - if errors.Is(c.LookupTokenErr, authn.ErrTokenNeedsRotation) { - c.Redirect(hs.Cfg.AppSubURL + "/") - return - } + if errors.Is(c.LookupTokenErr, authn.ErrTokenNeedsRotation) { + c.Redirect(hs.Cfg.AppSubURL + "/") + return } viewData, err := setIndexViewData(hs, c) diff --git a/pkg/api/user_token_test.go b/pkg/api/user_token_test.go index c520a52fea3..62fa0c2b0db 100644 --- a/pkg/api/user_token_test.go +++ b/pkg/api/user_token_test.go @@ -16,7 +16,6 @@ import ( "github.com/grafana/grafana/pkg/services/auth" "github.com/grafana/grafana/pkg/services/auth/authtest" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/services/user/usertest" @@ -200,7 +199,6 @@ func TestHTTPServer_RotateUserAuthToken(t *testing.T) { hs.Cfg = cfg hs.log = log.New() hs.Cfg.LoginCookieName = "grafana_session" - hs.Features = featuremgmt.WithFeatures(featuremgmt.FlagClientTokenRotation) hs.AuthTokenService = &authtest.FakeUserAuthTokenService{ RotateTokenProvider: func(ctx context.Context, cmd auth.RotateCommand) (*auth.UserToken, error) { return tt.rotatedToken, tt.rotatedErr diff --git a/pkg/services/authn/authnimpl/service.go b/pkg/services/authn/authnimpl/service.go index 4007dc1ece1..b436d098cfb 100644 --- a/pkg/services/authn/authnimpl/service.go +++ b/pkg/services/authn/authnimpl/service.go @@ -94,7 +94,7 @@ func ProvideService( s.RegisterClient(clients.ProvideAPIKey(apikeyService, userService)) if cfg.LoginCookieName != "" { - s.RegisterClient(clients.ProvideSession(cfg, sessionService, features)) + s.RegisterClient(clients.ProvideSession(cfg, sessionService)) } var proxyClients []authn.ProxyClient diff --git a/pkg/services/authn/clients/session.go b/pkg/services/authn/clients/session.go index 03339f193fa..1af47b16449 100644 --- a/pkg/services/authn/clients/session.go +++ b/pkg/services/authn/clients/session.go @@ -2,27 +2,20 @@ package clients import ( "context" - "errors" "net/url" "time" "github.com/grafana/grafana/pkg/infra/log" - "github.com/grafana/grafana/pkg/infra/network" "github.com/grafana/grafana/pkg/services/auth" "github.com/grafana/grafana/pkg/services/authn" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/setting" - "github.com/grafana/grafana/pkg/web" ) -var _ authn.HookClient = new(Session) var _ authn.ContextAwareClient = new(Session) -func ProvideSession(cfg *setting.Cfg, sessionService auth.UserTokenService, - features featuremgmt.FeatureToggles) *Session { +func ProvideSession(cfg *setting.Cfg, sessionService auth.UserTokenService) *Session { return &Session{ cfg: cfg, - features: features, sessionService: sessionService, log: log.New(authn.ClientSession), } @@ -30,7 +23,6 @@ func ProvideSession(cfg *setting.Cfg, sessionService auth.UserTokenService, type Session struct { cfg *setting.Cfg - features featuremgmt.FeatureToggles sessionService auth.UserTokenService log log.Logger } @@ -55,10 +47,8 @@ func (s *Session) Authenticate(ctx context.Context, r *authn.Request) (*authn.Id return nil, err } - if s.features.IsEnabled(ctx, featuremgmt.FlagClientTokenRotation) { - if token.NeedsRotation(time.Duration(s.cfg.TokenRotationIntervalMinutes) * time.Minute) { - return nil, authn.ErrTokenNeedsRotation.Errorf("token needs to be rotated") - } + if token.NeedsRotation(time.Duration(s.cfg.TokenRotationIntervalMinutes) * time.Minute) { + return nil, authn.ErrTokenNeedsRotation.Errorf("token needs to be rotated") } return &authn.Identity{ @@ -86,40 +76,3 @@ func (s *Session) Test(ctx context.Context, r *authn.Request) bool { func (s *Session) Priority() uint { return 60 } - -func (s *Session) Hook(ctx context.Context, identity *authn.Identity, r *authn.Request) error { - if identity.SessionToken == nil || s.features.IsEnabled(ctx, featuremgmt.FlagClientTokenRotation) { - return nil - } - - r.Resp.Before(func(w web.ResponseWriter) { - if w.Written() || errors.Is(ctx.Err(), context.Canceled) { - return - } - - // FIXME (jguer): get real values - addr := web.RemoteAddr(r.HTTPRequest) - userAgent := r.HTTPRequest.UserAgent() - - // addr := reqContext.RemoteAddr() - ip, err := network.GetIPFromAddress(addr) - if err != nil { - s.log.Debug("Failed to get client IP address", "addr", addr, "err", err) - ip = nil - } - rotated, newToken, err := s.sessionService.TryRotateToken(ctx, identity.SessionToken, ip, userAgent) - if err != nil { - s.log.Error("Failed to rotate token", "error", err) - return - } - - if rotated { - identity.SessionToken = newToken - s.log.Debug("Rotated session token", "user", identity.ID) - - authn.WriteSessionCookie(w, s.cfg, identity.SessionToken) - } - }) - - return nil -} diff --git a/pkg/services/authn/clients/session_test.go b/pkg/services/authn/clients/session_test.go index 7e8a3954f10..41fc4e5f7ae 100644 --- a/pkg/services/authn/clients/session_test.go +++ b/pkg/services/authn/clients/session_test.go @@ -2,7 +2,6 @@ package clients import ( "context" - "net" "net/http" "testing" "time" @@ -14,9 +13,7 @@ import ( "github.com/grafana/grafana/pkg/services/auth" "github.com/grafana/grafana/pkg/services/auth/authtest" "github.com/grafana/grafana/pkg/services/authn" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/setting" - "github.com/grafana/grafana/pkg/web" ) func TestSession_Test(t *testing.T) { @@ -29,7 +26,7 @@ func TestSession_Test(t *testing.T) { cfg := setting.NewCfg() cfg.LoginCookieName = "" cfg.LoginMaxLifetime = 20 * time.Second - s := ProvideSession(cfg, &authtest.FakeUserAuthTokenService{}, featuremgmt.WithFeatures()) + s := ProvideSession(cfg, &authtest.FakeUserAuthTokenService{}) disabled := s.Test(context.Background(), &authn.Request{HTTPRequest: validHTTPReq}) assert.False(t, disabled) @@ -64,7 +61,6 @@ func TestSession_Authenticate(t *testing.T) { type fields struct { sessionService auth.UserTokenService - features featuremgmt.FeatureToggles } type args struct { r *authn.Request @@ -80,7 +76,6 @@ func TestSession_Authenticate(t *testing.T) { name: "cookie not found", fields: fields{ sessionService: &authtest.FakeUserAuthTokenService{}, - features: featuremgmt.WithFeatures(), }, args: args{r: &authn.Request{HTTPRequest: &http.Request{}}}, wantID: nil, @@ -92,7 +87,6 @@ func TestSession_Authenticate(t *testing.T) { sessionService: &authtest.FakeUserAuthTokenService{LookupTokenProvider: func(ctx context.Context, unhashedToken string) (*auth.UserToken, error) { return validToken, nil }}, - features: featuremgmt.WithFeatures(), }, args: args{r: &authn.Request{HTTPRequest: validHTTPReq}}, wantID: &authn.Identity{ @@ -106,7 +100,7 @@ func TestSession_Authenticate(t *testing.T) { wantErr: false, }, { - name: "should return error for token that needs rotation if ClientTokenRotation is enabled", + name: "should return error for token that needs rotation", fields: fields{ sessionService: &authtest.FakeUserAuthTokenService{LookupTokenProvider: func(ctx context.Context, unhashedToken string) (*auth.UserToken, error) { return &auth.UserToken{ @@ -114,18 +108,16 @@ func TestSession_Authenticate(t *testing.T) { RotatedAt: time.Now().Add(-11 * time.Minute).Unix(), }, nil }}, - features: featuremgmt.WithFeatures(featuremgmt.FlagClientTokenRotation), }, args: args{r: &authn.Request{HTTPRequest: validHTTPReq}}, wantErr: true, }, { - name: "should return identity for token that don't need rotation if ClientTokenRotation is enabled", + name: "should return identity for token that don't need rotation", fields: fields{ sessionService: &authtest.FakeUserAuthTokenService{LookupTokenProvider: func(ctx context.Context, unhashedToken string) (*auth.UserToken, error) { return validToken, nil }}, - features: featuremgmt.WithFeatures(featuremgmt.FlagClientTokenRotation), }, args: args{r: &authn.Request{HTTPRequest: validHTTPReq}}, wantID: &authn.Identity{ @@ -145,7 +137,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) + s := ProvideSession(cfg, tt.fields.sessionService) got, err := s.Authenticate(context.Background(), tt.args.r) require.True(t, (err != nil) == tt.wantErr, err) @@ -157,73 +149,3 @@ func TestSession_Authenticate(t *testing.T) { }) } } - -type fakeResponseWriter struct { - Status int - HeaderStore http.Header -} - -func (f *fakeResponseWriter) Header() http.Header { - return f.HeaderStore -} - -func (f *fakeResponseWriter) Write([]byte) (int, error) { - return 0, nil -} - -func (f *fakeResponseWriter) WriteHeader(statusCode int) { - f.Status = statusCode -} - -func TestSession_Hook(t *testing.T) { - t.Run("should rotate token", func(t *testing.T) { - cfg := setting.NewCfg() - cfg.LoginCookieName = "grafana-session" - cfg.LoginMaxLifetime = 20 * time.Second - s := ProvideSession(cfg, &authtest.FakeUserAuthTokenService{ - TryRotateTokenProvider: func(ctx context.Context, token *auth.UserToken, clientIP net.IP, userAgent string) (bool, *auth.UserToken, error) { - token.UnhashedToken = "new-token" - return true, token, nil - }, - }, featuremgmt.WithFeatures()) - - sampleID := &authn.Identity{ - SessionToken: &auth.UserToken{ - Id: 1, - UserId: 1, - }, - } - - mockResponseWriter := &fakeResponseWriter{ - Status: 0, - HeaderStore: map[string][]string{}, - } - - resp := &authn.Request{ - HTTPRequest: &http.Request{ - Header: map[string][]string{}, - }, - Resp: web.NewResponseWriter(http.MethodConnect, mockResponseWriter), - } - - err := s.Hook(context.Background(), sampleID, resp) - require.NoError(t, err) - - resp.Resp.WriteHeader(201) - require.Equal(t, 201, mockResponseWriter.Status) - - assert.Equal(t, "new-token", sampleID.SessionToken.UnhashedToken) - require.Len(t, mockResponseWriter.HeaderStore, 1) - assert.Equal(t, "grafana-session=new-token; Path=/; Max-Age=20; HttpOnly", - mockResponseWriter.HeaderStore.Get("set-cookie"), mockResponseWriter.HeaderStore) - }) - - t.Run("should not rotate token with feature flag", func(t *testing.T) { - s := ProvideSession(setting.NewCfg(), nil, featuremgmt.WithFeatures(featuremgmt.FlagClientTokenRotation)) - - req := &authn.Request{} - identity := &authn.Identity{} - err := s.Hook(context.Background(), identity, req) - require.NoError(t, err) - }) -} diff --git a/pkg/services/contexthandler/contexthandler.go b/pkg/services/contexthandler/contexthandler.go index 7bb582c970e..256d13a34fa 100644 --- a/pkg/services/contexthandler/contexthandler.go +++ b/pkg/services/contexthandler/contexthandler.go @@ -3,7 +3,6 @@ package contexthandler import ( "context" - "errors" "fmt" "net/http" @@ -13,7 +12,6 @@ import ( "github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" - "github.com/grafana/grafana/pkg/services/auth" "github.com/grafana/grafana/pkg/services/auth/identity" "github.com/grafana/grafana/pkg/services/authn" "github.com/grafana/grafana/pkg/services/contexthandler/ctxkey" @@ -115,11 +113,6 @@ func (h *ContextHandler) Middleware(next http.Handler) http.Handler { identity, err := h.authnService.Authenticate(reqContext.Req.Context(), &authn.Request{HTTPRequest: reqContext.Req, Resp: reqContext.Resp}) if err != nil { - if errors.Is(err, auth.ErrInvalidSessionToken) || errors.Is(err, authn.ErrExpiredAccessToken) { - // Burn the cookie in case of invalid, expired or missing token - reqContext.Resp.Before(h.deleteInvalidCookieEndOfRequestFunc(reqContext)) - } - // Hack: set all errors on LookupTokenErr, so we can check it in auth middlewares reqContext.LookupTokenErr = err } else { @@ -170,22 +163,6 @@ func (h *ContextHandler) addIDHeaderEndOfRequestFunc(ident identity.Requester) w } } -func (h *ContextHandler) deleteInvalidCookieEndOfRequestFunc(reqContext *contextmodel.ReqContext) web.BeforeFunc { - return func(w web.ResponseWriter) { - if h.features.IsEnabled(reqContext.Req.Context(), featuremgmt.FlagClientTokenRotation) { - return - } - - if w.Written() { - reqContext.Logger.Debug("Response written, skipping invalid cookie delete") - return - } - - reqContext.Logger.Debug("Expiring invalid cookie") - authn.DeleteSessionCookie(reqContext.Resp, h.Cfg) - } -} - type authHTTPHeaderListContextKey struct{} var authHTTPHeaderListKey = authHTTPHeaderListContextKey{} diff --git a/pkg/services/contexthandler/contexthandler_test.go b/pkg/services/contexthandler/contexthandler_test.go index cd6b1e1663b..6662115355a 100644 --- a/pkg/services/contexthandler/contexthandler_test.go +++ b/pkg/services/contexthandler/contexthandler_test.go @@ -10,7 +10,6 @@ import ( "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/infra/tracing" - "github.com/grafana/grafana/pkg/services/auth" "github.com/grafana/grafana/pkg/services/authn" "github.com/grafana/grafana/pkg/services/authn/authntest" "github.com/grafana/grafana/pkg/services/contexthandler" @@ -111,46 +110,6 @@ func TestContextHandler(t *testing.T) { require.NoError(t, res.Body.Close()) }) - t.Run("should delete session cookie on invalid session", func(t *testing.T) { - handler := contexthandler.ProvideService( - setting.NewCfg(), - tracing.InitializeTracerForTest(), - featuremgmt.WithFeatures(), - &authntest.FakeService{ExpectedErr: auth.ErrInvalidSessionToken}, - ) - - server := webtest.NewServer(t, routing.NewRouteRegister()) - server.Mux.Use(handler.Middleware) - server.Mux.Get("/api/handler", func(c *contextmodel.ReqContext) {}) - - res, err := server.Send(server.NewGetRequest("/api/handler")) - require.NoError(t, err) - cookies := res.Cookies() - require.Len(t, cookies, 1) - require.Equal(t, cookies[0].String(), "grafana_session_expiry=; Path=/; Max-Age=0") - require.NoError(t, res.Body.Close()) - }) - - t.Run("should delete session cookie when oauth token refresh failed", func(t *testing.T) { - handler := contexthandler.ProvideService( - setting.NewCfg(), - tracing.InitializeTracerForTest(), - featuremgmt.WithFeatures(), - &authntest.FakeService{ExpectedErr: authn.ErrExpiredAccessToken.Errorf("")}, - ) - - server := webtest.NewServer(t, routing.NewRouteRegister()) - server.Mux.Use(handler.Middleware) - server.Mux.Get("/api/handler", func(c *contextmodel.ReqContext) {}) - - res, err := server.Send(server.NewGetRequest("/api/handler")) - require.NoError(t, err) - cookies := res.Cookies() - require.Len(t, cookies, 1) - require.Equal(t, cookies[0].String(), "grafana_session_expiry=; Path=/; Max-Age=0") - require.NoError(t, res.Body.Close()) - }) - t.Run("should store auth header in context", func(t *testing.T) { cfg := setting.NewCfg() cfg.JWTAuth.Enabled = true diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index 53f52a327d1..e3a126ae461 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -382,14 +382,6 @@ var ( FrontendOnly: false, Owner: grafanaObservabilityMetricsSquad, }, - { - Name: "clientTokenRotation", - Description: "Replaces the current in-request token rotation so that the client initiates the rotation", - Stage: FeatureStageGeneralAvailability, - Expression: "true", - Owner: identityAccessTeam, - AllowSelfServe: false, - }, { Name: "prometheusDataplane", Description: "Changes responses to from Prometheus to be compliant with the dataplane specification. In particular, when this feature toggle is active, the numeric `Field.Name` is set from 'Value' to the value of the `__name__` label.", diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index d4b46223ef9..18a3f9cef5a 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -49,7 +49,6 @@ prometheusMetricEncyclopedia,GA,@grafana/observability-metrics,false,false,true influxdbBackendMigration,GA,@grafana/observability-metrics,false,false,true influxqlStreamingParser,experimental,@grafana/observability-metrics,false,false,false influxdbRunQueriesInParallel,privatePreview,@grafana/observability-metrics,false,false,false -clientTokenRotation,GA,@grafana/identity-access-team,false,false,false prometheusDataplane,GA,@grafana/observability-metrics,false,false,false lokiMetricDataplane,GA,@grafana/observability-logs,false,false,false lokiLogsDataplane,experimental,@grafana/observability-logs,false,false,false diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index 70fcfc2c6b6..caa95fab64a 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -207,10 +207,6 @@ const ( // Enables running InfluxDB Influxql queries in parallel FlagInfluxdbRunQueriesInParallel = "influxdbRunQueriesInParallel" - // FlagClientTokenRotation - // Replaces the current in-request token rotation so that the client initiates the rotation - FlagClientTokenRotation = "clientTokenRotation" - // FlagPrometheusDataplane // Changes responses to from Prometheus to be compliant with the dataplane specification. In particular, when this feature toggle is active, the numeric `Field.Name` is set from 'Value' to the value of the `__name__` label. FlagPrometheusDataplane = "prometheusDataplane" diff --git a/pkg/services/featuremgmt/toggles_gen.json b/pkg/services/featuremgmt/toggles_gen.json index 3e5ffb8f7fe..670f4929bd8 100644 --- a/pkg/services/featuremgmt/toggles_gen.json +++ b/pkg/services/featuremgmt/toggles_gen.json @@ -1287,7 +1287,8 @@ "metadata": { "name": "clientTokenRotation", "resourceVersion": "1707928895402", - "creationTimestamp": "2024-02-14T16:41:35Z" + "creationTimestamp": "2024-02-14T16:41:35Z", + "deletionTimestamp": "2024-02-16T10:26:09Z" }, "spec": { "description": "Replaces the current in-request token rotation so that the client initiates the rotation", diff --git a/public/app/core/services/backend_srv.ts b/public/app/core/services/backend_srv.ts index 9bbf41c9fa9..331f0b00ab2 100644 --- a/public/app/core/services/backend_srv.ts +++ b/public/app/core/services/backend_srv.ts @@ -387,7 +387,7 @@ export class BackendSrv implements BackendService { let authChecker = this.loginPing(); const expired = getSessionExpiry() * 1000 < Date.now(); - if (config.featureToggles.clientTokenRotation && expired) { + if (expired) { authChecker = this.rotateToken(); } diff --git a/public/app/core/services/context_srv.ts b/public/app/core/services/context_srv.ts index d305d2a4fbc..ab1016fd71a 100644 --- a/public/app/core/services/context_srv.ts +++ b/public/app/core/services/context_srv.ts @@ -211,11 +211,6 @@ export class ContextSrv { return false; } - // skip if feature toggle is not enabled - if (!config.featureToggles.clientTokenRotation) { - return false; - } - // skip if there is no session to rotate // if a user has a session but not yet a session expiry cookie, can happen during upgrade // from an older version of grafana, we never schedule the job and the fallback logic @@ -229,7 +224,7 @@ export class ContextSrv { } private cancelTokenRotationJob() { - if (config.featureToggles.clientTokenRotation && this.tokenRotationJobId > 0) { + if (this.tokenRotationJobId > 0) { clearTimeout(this.tokenRotationJobId); } } diff --git a/public/app/core/specs/backend_srv.test.ts b/public/app/core/specs/backend_srv.test.ts index fad3b7e8665..25dd30124f8 100644 --- a/public/app/core/specs/backend_srv.test.ts +++ b/public/app/core/specs/backend_srv.test.ts @@ -4,7 +4,7 @@ import { fromFetch } from 'rxjs/fetch'; import { delay } from 'rxjs/operators'; import { AppEvents, DataQueryErrorType, EventBusExtended } from '@grafana/data'; -import { BackendSrvRequest, FetchError, config, FetchResponse } from '@grafana/runtime'; +import { BackendSrvRequest, FetchError, FetchResponse } from '@grafana/runtime'; import { TokenRevokedModal } from '../../features/users/TokenRevokedModal'; import { ShowModalReactEvent } from '../../types/events'; @@ -158,27 +158,19 @@ describe('backendSrv', () => { }); }); - describe('when making an unsuccessful call and conditions for retry are favorable and loginPing does not throw', () => { + describe('when making an unsuccessful call and conditions for retry are favorable and rotateToken does not throw', () => { const url = '/api/dashboard/'; const okResponse = { ok: true, status: 200, statusText: 'OK', data: { message: 'Ok' } }; - let fetchMock: jest.SpyInstance; + let fetchMock: jest.SpyInstance; afterEach(() => { fetchMock.mockClear(); }); - afterAll(() => { fetchMock.mockRestore(); - config.featureToggles.clientTokenRotation = false; }); - it.each` - clientTokenRotation - ${true} - ${false} - `('then it should retry (clientTokenRotation = %s)', async ({ clientTokenRotation }) => { - config.featureToggles.clientTokenRotation = clientTokenRotation; - + it('then it should retry', async () => { fetchMock = jest .spyOn(global, 'fetch') .mockRejectedValueOnce({ @@ -210,19 +202,13 @@ describe('backendSrv', () => { false ); - backendSrv.loginPing = jest.fn().mockResolvedValue(okResponse); - backendSrv.rotateToken = jest.fn().mockResolvedValue(okResponse); await backendSrv.request({ url, method: 'GET', retry: 0 }).finally(() => { expect(appEventsMock.emit).not.toHaveBeenCalled(); expect(logoutMock).not.toHaveBeenCalled(); - if (config.featureToggles.clientTokenRotation) { - expect(backendSrv.rotateToken).toHaveBeenCalledTimes(1); - } else { - expect(backendSrv.loginPing).toHaveBeenCalledTimes(1); - } - expect(fetchMock).toHaveBeenCalledTimes(2); // expecting 2 calls because of retry and because the loginPing/tokenRotation is mocked + expect(backendSrv.rotateToken).toHaveBeenCalledTimes(1); + expect(fetchMock).toHaveBeenCalledTimes(2); // expecting 2 calls because of retry and because the tokenRotation is mocked }); }); }); @@ -238,7 +224,7 @@ describe('backendSrv', () => { url, }); - backendSrv.loginPing = jest.fn(); + backendSrv.rotateToken = jest.fn(); await backendSrv.request({ url, method: 'GET', retry: 0 }).catch(() => { expect(appEventsMock.publish).toHaveBeenCalledTimes(1); @@ -250,7 +236,7 @@ describe('backendSrv', () => { }, }) ); - expect(backendSrv.loginPing).not.toHaveBeenCalled(); + expect(backendSrv.rotateToken).not.toHaveBeenCalled(); expect(logoutMock).not.toHaveBeenCalled(); expectRequestCallChain({ url, method: 'GET', retry: 0 }); }); @@ -267,7 +253,7 @@ describe('backendSrv', () => { data: { message: errorMessage }, }); - backendSrv.loginPing = jest + backendSrv.rotateToken = jest .fn() .mockRejectedValue({ status: 403, statusText: 'Forbidden', data: { message: 'Forbidden' } }); const url = '/api/dashboard/'; @@ -279,7 +265,7 @@ describe('backendSrv', () => { expect(error.statusText).toBe('Forbidden'); expect(error.data).toEqual({ message: 'Forbidden' }); expect(appEventsMock.emit).not.toHaveBeenCalled(); - expect(backendSrv.loginPing).toHaveBeenCalledTimes(1); + expect(backendSrv.rotateToken).toHaveBeenCalledTimes(1); expect(logoutMock).not.toHaveBeenCalled(); expectRequestCallChain({ url, method: 'GET', retry: 0 }); jest.advanceTimersByTime(50); @@ -519,27 +505,19 @@ describe('backendSrv', () => { }); }); - describe('when making an unsuccessful call and conditions for retry are favorable and loginPing does not throw', () => { + describe('when making an unsuccessful call and conditions for retry are favorable and rotateToken does not throw', () => { const url = '/api/dashboard/'; const okResponse = { ok: true, status: 200, statusText: 'OK', data: { message: 'Ok' } }; - let fetchMock: jest.SpyInstance; + let fetchMock: jest.SpyInstance; afterEach(() => { fetchMock.mockClear(); }); - afterAll(() => { fetchMock.mockRestore(); - config.featureToggles.clientTokenRotation = false; }); - it.each` - clientTokenRotation - ${true} - ${false} - `('then it should retry (clientTokenRotation = %s)', async ({ clientTokenRotation }) => { - config.featureToggles.clientTokenRotation = clientTokenRotation; - + it('then it should retry', async () => { fetchMock = jest .spyOn(global, 'fetch') .mockRejectedValueOnce({ @@ -570,18 +548,12 @@ describe('backendSrv', () => { false ); - backendSrv.loginPing = jest.fn().mockResolvedValue(okResponse); - backendSrv.rotateToken = jest.fn().mockResolvedValue(okResponse); await backendSrv.datasourceRequest({ url, method: 'GET', retry: 0 }).finally(() => { expect(logoutMock).not.toHaveBeenCalled(); - if (config.featureToggles.clientTokenRotation) { - expect(backendSrv.rotateToken).toHaveBeenCalledTimes(1); - } else { - expect(backendSrv.loginPing).toHaveBeenCalledTimes(1); - } - expect(fetchMock).toHaveBeenCalledTimes(2); // expecting 2 calls because of retry and because the loginPing/tokenRotation is mocked + expect(backendSrv.rotateToken).toHaveBeenCalledTimes(1); + expect(fetchMock).toHaveBeenCalledTimes(2); // expecting 2 calls because of retry and because the tokenRotation is mocked }); }); }); @@ -595,7 +567,7 @@ describe('backendSrv', () => { data: { message: 'Token revoked', error: { id: 'ERR_TOKEN_REVOKED', maxConcurrentSessions: 3 } }, }); - backendSrv.loginPing = jest.fn(); + backendSrv.rotateToken = jest.fn(); const url = '/api/dashboard/'; @@ -609,7 +581,7 @@ describe('backendSrv', () => { }, }) ); - expect(backendSrv.loginPing).not.toHaveBeenCalled(); + expect(backendSrv.rotateToken).not.toHaveBeenCalled(); expect(logoutMock).not.toHaveBeenCalled(); expectRequestCallChain({ url, method: 'GET', retry: 0 }); }); @@ -631,7 +603,7 @@ describe('backendSrv', () => { retry: 0, }; - backendSrv.loginPing = jest + backendSrv.rotateToken = jest .fn() .mockRejectedValue({ status: 403, statusText: 'Forbidden', data: { message: 'Forbidden' } }); @@ -639,7 +611,7 @@ describe('backendSrv', () => { expect(error.status).toBe(403); expect(error.statusText).toBe('Forbidden'); expect(error.data).toEqual({ message: 'Forbidden' }); - expect(backendSrv.loginPing).toHaveBeenCalledTimes(1); + expect(backendSrv.rotateToken).toHaveBeenCalledTimes(1); expect(logoutMock).not.toHaveBeenCalled(); expectRequestCallChain(options); });