OAuth: Feature toggle for access token expiration check and docs (#58179)
* Add feature toggle for access token expiration check * Add docs for configuring refresh tokens * Update docs * Update docs based on review Co-authored-by: Christopher Moyer <35463610+chri2547@users.noreply.github.com> * Improve documentation * Change access_type default to Offline * Update docs/sources/setup-grafana/configure-security/configure-authentication/gitlab/index.md Co-authored-by: Christopher Moyer <35463610+chri2547@users.noreply.github.com> * Update docs/sources/setup-grafana/configure-security/configure-authentication/google/index.md Co-authored-by: Christopher Moyer <35463610+chri2547@users.noreply.github.com> * Update pkg/services/featuremgmt/registry.go Co-authored-by: Eric Leijonmarck <eric.leijonmarck@gmail.com> * Regenerate toggles * Update Generic OAuth docs Co-authored-by: Christopher Moyer <35463610+chri2547@users.noreply.github.com> Co-authored-by: Eric Leijonmarck <eric.leijonmarck@gmail.com>
This commit is contained in:
co-authored by
Christopher Moyer
Eric Leijonmarck
parent
a9458c8c00
commit
4915d21c25
@@ -215,7 +215,7 @@ func getContextHandler(t *testing.T, cfg *setting.Cfg) *contexthandler.ContextHa
|
||||
authProxy := authproxy.ProvideAuthProxy(cfg, remoteCacheSvc, loginservice.LoginServiceMock{}, &usertest.FakeUserService{}, sqlStore)
|
||||
loginService := &logintest.LoginServiceFake{}
|
||||
authenticator := &logintest.AuthenticatorFake{}
|
||||
ctxHdlr := contexthandler.ProvideService(cfg, userAuthTokenSvc, authJWTSvc, remoteCacheSvc, renderSvc, sqlStore, tracer, authProxy, loginService, nil, authenticator, usertest.NewUserServiceFake(), orgtest.NewOrgServiceFake(), nil)
|
||||
ctxHdlr := contexthandler.ProvideService(cfg, userAuthTokenSvc, authJWTSvc, remoteCacheSvc, renderSvc, sqlStore, tracer, authProxy, loginService, nil, authenticator, usertest.NewUserServiceFake(), orgtest.NewOrgServiceFake(), nil, featuremgmt.WithFeatures())
|
||||
|
||||
return ctxHdlr
|
||||
}
|
||||
|
||||
@@ -97,7 +97,8 @@ func (hs *HTTPServer) OAuthLogin(ctx *models.ReqContext) {
|
||||
|
||||
code := ctx.Query("code")
|
||||
if code == "" {
|
||||
opts := []oauth2.AuthCodeOption{oauth2.AccessTypeOnline}
|
||||
// FIXME: access_type is a Google OAuth2 specific thing, consider refactoring this and moving to google_oauth.go
|
||||
opts := []oauth2.AuthCodeOption{oauth2.AccessTypeOffline}
|
||||
|
||||
if provider.UsePKCE {
|
||||
ascii, pkce, err := genPKCECode()
|
||||
|
||||
@@ -30,6 +30,7 @@ import (
|
||||
"github.com/grafana/grafana/pkg/services/auth"
|
||||
"github.com/grafana/grafana/pkg/services/contexthandler"
|
||||
"github.com/grafana/grafana/pkg/services/contexthandler/authproxy"
|
||||
"github.com/grafana/grafana/pkg/services/featuremgmt"
|
||||
"github.com/grafana/grafana/pkg/services/login/loginservice"
|
||||
"github.com/grafana/grafana/pkg/services/login/logintest"
|
||||
"github.com/grafana/grafana/pkg/services/navtree"
|
||||
@@ -832,7 +833,7 @@ func getContextHandler(t *testing.T, cfg *setting.Cfg, mockSQLStore *dbtest.Fake
|
||||
tracer := tracing.InitializeTracerForTest()
|
||||
authProxy := authproxy.ProvideAuthProxy(cfg, remoteCacheSvc, loginService, userService, mockSQLStore)
|
||||
authenticator := &logintest.AuthenticatorFake{ExpectedUser: &user.User{}}
|
||||
return contexthandler.ProvideService(cfg, userAuthTokenSvc, authJWTSvc, remoteCacheSvc, renderSvc, mockSQLStore, tracer, authProxy, loginService, apiKeyService, authenticator, userService, orgService, oauthTokenService)
|
||||
return contexthandler.ProvideService(cfg, userAuthTokenSvc, authJWTSvc, remoteCacheSvc, renderSvc, mockSQLStore, tracer, authProxy, loginService, apiKeyService, authenticator, userService, orgService, oauthTokenService, featuremgmt.WithFeatures(featuremgmt.FlagAccessTokenExpirationCheck))
|
||||
}
|
||||
|
||||
type fakeRenderService struct {
|
||||
|
||||
@@ -104,7 +104,7 @@ func getContextHandler(t *testing.T) *ContextHandler {
|
||||
|
||||
return ProvideService(cfg, userAuthTokenSvc, authJWTSvc, remoteCacheSvc,
|
||||
renderSvc, sqlStore, tracer, authProxy, loginService, nil, authenticator,
|
||||
&userService, orgService, nil)
|
||||
&userService, orgService, nil, nil)
|
||||
}
|
||||
|
||||
type FakeGetSignUserStore struct {
|
||||
|
||||
@@ -24,6 +24,7 @@ import (
|
||||
"github.com/grafana/grafana/pkg/services/apikey"
|
||||
"github.com/grafana/grafana/pkg/services/contexthandler/authproxy"
|
||||
"github.com/grafana/grafana/pkg/services/contexthandler/ctxkey"
|
||||
"github.com/grafana/grafana/pkg/services/featuremgmt"
|
||||
"github.com/grafana/grafana/pkg/services/login"
|
||||
"github.com/grafana/grafana/pkg/services/oauthtoken"
|
||||
"github.com/grafana/grafana/pkg/services/org"
|
||||
@@ -46,7 +47,7 @@ func ProvideService(cfg *setting.Cfg, tokenService models.UserTokenService, jwtS
|
||||
remoteCache *remotecache.RemoteCache, renderService rendering.Service, sqlStore db.DB,
|
||||
tracer tracing.Tracer, authProxy *authproxy.AuthProxy, loginService login.Service,
|
||||
apiKeyService apikey.Service, authenticator loginpkg.Authenticator, userService user.Service,
|
||||
orgService org.Service, oauthTokenService oauthtoken.OAuthTokenService,
|
||||
orgService org.Service, oauthTokenService oauthtoken.OAuthTokenService, features *featuremgmt.FeatureManager,
|
||||
) *ContextHandler {
|
||||
return &ContextHandler{
|
||||
Cfg: cfg,
|
||||
@@ -63,6 +64,7 @@ func ProvideService(cfg *setting.Cfg, tokenService models.UserTokenService, jwtS
|
||||
userService: userService,
|
||||
orgService: orgService,
|
||||
oauthTokenService: oauthTokenService,
|
||||
features: features,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -82,6 +84,7 @@ type ContextHandler struct {
|
||||
userService user.Service
|
||||
orgService org.Service
|
||||
oauthTokenService oauthtoken.OAuthTokenService
|
||||
features *featuremgmt.FeatureManager
|
||||
// GetTime returns the current time.
|
||||
// Stubbable by tests.
|
||||
GetTime func() time.Time
|
||||
@@ -445,29 +448,31 @@ func (h *ContextHandler) initContextWithToken(reqContext *models.ReqContext, org
|
||||
getTime = time.Now
|
||||
}
|
||||
|
||||
// Check whether the logged in User has a token (whether the User used an OAuth provider to login)
|
||||
oauthToken, exists, _ := h.oauthTokenService.HasOAuthEntry(ctx, queryResult)
|
||||
if exists {
|
||||
// Skip where the OAuthExpiry is default/zero/unset
|
||||
if !oauthToken.OAuthExpiry.IsZero() && oauthToken.OAuthExpiry.Round(0).Add(-oauthtoken.ExpiryDelta).Before(getTime()) {
|
||||
reqContext.Logger.Info("access token expired", "userId", query.UserID, "expiry", fmt.Sprintf("%v", oauthToken.OAuthExpiry))
|
||||
if h.features.IsEnabled(featuremgmt.FlagAccessTokenExpirationCheck) {
|
||||
// Check whether the logged in User has a token (whether the User used an OAuth provider to login)
|
||||
oauthToken, exists, _ := h.oauthTokenService.HasOAuthEntry(ctx, queryResult)
|
||||
if exists {
|
||||
// Skip where the OAuthExpiry is default/zero/unset
|
||||
if !oauthToken.OAuthExpiry.IsZero() && oauthToken.OAuthExpiry.Round(0).Add(-oauthtoken.ExpiryDelta).Before(getTime()) {
|
||||
reqContext.Logger.Info("access token expired", "userId", query.UserID, "expiry", fmt.Sprintf("%v", oauthToken.OAuthExpiry))
|
||||
|
||||
// If the User doesn't have a refresh_token or refreshing the token was unsuccessful then log out the User and Invalidate the OAuth tokens
|
||||
if err = h.oauthTokenService.TryTokenRefresh(ctx, oauthToken); err != nil {
|
||||
if !errors.Is(err, oauthtoken.ErrNoRefreshTokenFound) {
|
||||
reqContext.Logger.Error("could not fetch a new access token", "userId", oauthToken.UserId, "error", err)
|
||||
}
|
||||
// If the User doesn't have a refresh_token or refreshing the token was unsuccessful then log out the User and Invalidate the OAuth tokens
|
||||
if err = h.oauthTokenService.TryTokenRefresh(ctx, oauthToken); err != nil {
|
||||
if !errors.Is(err, oauthtoken.ErrNoRefreshTokenFound) {
|
||||
reqContext.Logger.Error("could not fetch a new access token", "userId", oauthToken.UserId, "error", err)
|
||||
}
|
||||
|
||||
reqContext.Resp.Before(h.deleteInvalidCookieEndOfRequestFunc(reqContext))
|
||||
if err = h.oauthTokenService.InvalidateOAuthTokens(ctx, oauthToken); err != nil {
|
||||
reqContext.Logger.Error("could not invalidate OAuth tokens", "userId", oauthToken.UserId, "error", err)
|
||||
}
|
||||
reqContext.Resp.Before(h.deleteInvalidCookieEndOfRequestFunc(reqContext))
|
||||
if err = h.oauthTokenService.InvalidateOAuthTokens(ctx, oauthToken); err != nil {
|
||||
reqContext.Logger.Error("could not invalidate OAuth tokens", "userId", oauthToken.UserId, "error", err)
|
||||
}
|
||||
|
||||
err = h.AuthTokenService.RevokeToken(ctx, token, false)
|
||||
if err != nil && !errors.Is(err, models.ErrUserTokenNotFound) {
|
||||
reqContext.Logger.Error("failed to revoke auth token", "error", err)
|
||||
err = h.AuthTokenService.RevokeToken(ctx, token, false)
|
||||
if err != nil && !errors.Is(err, models.ErrUserTokenNotFound) {
|
||||
reqContext.Logger.Error("failed to revoke auth token", "error", err)
|
||||
}
|
||||
return false
|
||||
}
|
||||
return false
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -357,6 +357,11 @@ var (
|
||||
State: FeatureStateAlpha,
|
||||
RequiresDevMode: true,
|
||||
},
|
||||
{
|
||||
Name: "accessTokenExpirationCheck",
|
||||
Description: "Enable OAuth access_token expiration check and token refresh using the refresh_token",
|
||||
State: FeatureStateStable,
|
||||
},
|
||||
{
|
||||
Name: "elasticsearchBackendMigration",
|
||||
Description: "Use Elasticsearch as backend data source",
|
||||
|
||||
@@ -263,6 +263,10 @@ const (
|
||||
// Enable folder nesting
|
||||
FlagNestedFolders = "nestedFolders"
|
||||
|
||||
// FlagAccessTokenExpirationCheck
|
||||
// Enable OAuth access_token expiration check and token refresh using the refresh_token
|
||||
FlagAccessTokenExpirationCheck = "accessTokenExpirationCheck"
|
||||
|
||||
// FlagElasticsearchBackendMigration
|
||||
// Use Elasticsearch as backend data source
|
||||
FlagElasticsearchBackendMigration = "elasticsearchBackendMigration"
|
||||
|
||||
Reference in New Issue
Block a user