From 4915d21c25891dae2ee7a4eec4fe0dd2659d614d Mon Sep 17 00:00:00 2001 From: Misi Date: Mon, 14 Nov 2022 16:47:46 +0100 Subject: [PATCH] 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 * Regenerate toggles * Update Generic OAuth docs Co-authored-by: Christopher Moyer <35463610+chri2547@users.noreply.github.com> Co-authored-by: Eric Leijonmarck --- .../configure-authentication/azuread/index.md | 12 +++++ .../generic-oauth/index.md | 23 +++++++++- .../configure-authentication/github/index.md | 8 ++++ .../configure-authentication/gitlab/index.md | 12 +++++ .../configure-authentication/google/index.md | 12 +++++ .../keycloak/index.md | 12 +++++ .../configure-authentication/okta/index.md | 13 ++++++ .../src/types/featureToggles.gen.ts | 1 + pkg/api/common_test.go | 2 +- pkg/api/login_oauth.go | 3 +- pkg/middleware/middleware_test.go | 3 +- .../contexthandler/auth_proxy_test.go | 2 +- pkg/services/contexthandler/contexthandler.go | 45 ++++++++++--------- pkg/services/featuremgmt/registry.go | 5 +++ pkg/services/featuremgmt/toggles_gen.go | 4 ++ 15 files changed, 131 insertions(+), 26 deletions(-) diff --git a/docs/sources/setup-grafana/configure-security/configure-authentication/azuread/index.md b/docs/sources/setup-grafana/configure-security/configure-authentication/azuread/index.md index 3932b82a7e0..42d1760231a 100644 --- a/docs/sources/setup-grafana/configure-security/configure-authentication/azuread/index.md +++ b/docs/sources/setup-grafana/configure-security/configure-authentication/azuread/index.md @@ -169,6 +169,18 @@ GF_AUTH_AZUREAD_CLIENT_SECRET **Note:** Verify that the Grafana [root_url]({{< relref "../../../configure-grafana/#root-url" >}}) is set in your Azure Application Redirect URLs. +### Configure refresh token + +> Available in Grafana v9.3 and later versions. + +> **Note:** This feature is behind the `accessTokenExpirationCheck` feature toggle. + +When a user logs in using an OAuth provider, Grafana verifies that the access token has not expired. When an access token expires, Grafana uses the provided refresh token (if any exists) to obtain a new access token. + +Grafana uses a refresh token to obtain a new access token without requiring the user to log in again. If a refresh token doesn't exist, Grafana logs the user out of the system after the access token has expired. + +To enable a refresh token for AzureAD, extend the `scopes` in `[auth.azuread]` with `offline_access`. + ### Configure allowed groups To limit access to authenticated users who are members of one or more groups, set `allowed_groups` diff --git a/docs/sources/setup-grafana/configure-security/configure-authentication/generic-oauth/index.md b/docs/sources/setup-grafana/configure-security/configure-authentication/generic-oauth/index.md index 90fce67d538..879b15cb51c 100644 --- a/docs/sources/setup-grafana/configure-security/configure-authentication/generic-oauth/index.md +++ b/docs/sources/setup-grafana/configure-security/configure-authentication/generic-oauth/index.md @@ -116,9 +116,24 @@ use_pkce = true Grafana always uses the SHA256 based `S256` challenge method and a 128 bytes (base64url encoded) code verifier. +### Configure refresh token + +> Available in Grafana v9.3 and later versions. + +> **Note:** This feature is behind the `accessTokenExpirationCheck` feature toggle. + +When a user logs in using an OAuth provider, Grafana verifies that the access token has not expired. When an access token expires, Grafana uses the provided refresh token (if any exists) to obtain a new access token. + +Grafana uses a refresh token to obtain a new access token without requiring the user to log in again. If a refresh token doesn't exist, Grafana logs the user out of the system after the access token has expired. + +To configure Generic OAuth to use a refresh token, perform one or both of the following tasks, if required: + +- Extend the `[auth.generic_oauth]` section with additional scopes +- Enable the refresh token on the provider + ## Set up OAuth2 with Auth0 -1. Create a new Client in Auth0 +1. Use the following parameters to create a client in Auth0: - Name: Grafana - Type: Regular Web Application @@ -138,7 +153,7 @@ Grafana always uses the SHA256 based `S256` challenge method and a 128 bytes (ba name = Auth0 client_id = client_secret = - scopes = openid profile email + scopes = openid profile email offline_access auth_url = https:///authorize token_url = https:///oauth/token api_url = https:///userinfo @@ -164,6 +179,8 @@ team_ids = allowed_organizations = ``` +By default, a refresh token is included in the response for the **Authorization Code Grant**. + ## Set up OAuth2 with Centrify 1. Create a new Custom OpenID Connect application configuration in the Centrify dashboard. @@ -195,6 +212,8 @@ allowed_organizations = api_url = https://.my.centrify.com/OAuth2/UserInfo/ ``` +By default, a refresh token is included in the response for the **Authorization Code Grant**. + ## Set up OAuth2 with OneLogin 1. Create a new Custom Connector with the following settings: diff --git a/docs/sources/setup-grafana/configure-security/configure-authentication/github/index.md b/docs/sources/setup-grafana/configure-security/configure-authentication/github/index.md index 1f8c90fb14e..dad8809d78e 100644 --- a/docs/sources/setup-grafana/configure-security/configure-authentication/github/index.md +++ b/docs/sources/setup-grafana/configure-security/configure-authentication/github/index.md @@ -64,6 +64,14 @@ automatically signed up. You can also use [variable expansion]({{< relref "../../../configure-grafana/#variable-expansion" >}}) to reference environment variables and local files in your GitHub auth configuration. +### GitHub refresh token + +> Available in Grafana v9.3 and later versions. + +> **Note:** This feature is behind the `accessTokenExpirationCheck` feature toggle. + +GitHub OAuth applications do not support refresh tokens because the provided access tokens do not expire. + ### team_ids Require an active team membership for at least one of the given teams on diff --git a/docs/sources/setup-grafana/configure-security/configure-authentication/gitlab/index.md b/docs/sources/setup-grafana/configure-security/configure-authentication/gitlab/index.md index 78a4049b498..f738c640e7a 100644 --- a/docs/sources/setup-grafana/configure-security/configure-authentication/gitlab/index.md +++ b/docs/sources/setup-grafana/configure-security/configure-authentication/gitlab/index.md @@ -81,6 +81,18 @@ to login on your Grafana instance. You can limit access to only members of a given group or list of groups by setting the `allowed_groups` option. +### Configure refresh token + +> Available in Grafana v9.3 and later versions. + +> **Note:** This feature is behind the `accessTokenExpirationCheck` feature toggle. + +When a user logs in using an OAuth provider, Grafana verifies that the access token has not expired. When an access token expires, Grafana uses the provided refresh token (if any exists) to obtain a new access token. + +Grafana uses a refresh token to obtain a new access token without requiring the user to log in again. If a refresh token doesn't exist, Grafana logs the user out of the system after the access token has expired. + +By default, GitLab provides a refresh token. + ### allowed_groups To limit access to authenticated users that are members of one or more [GitLab diff --git a/docs/sources/setup-grafana/configure-security/configure-authentication/google/index.md b/docs/sources/setup-grafana/configure-security/configure-authentication/google/index.md index 5af9b6fda29..91ef70ae5e1 100644 --- a/docs/sources/setup-grafana/configure-security/configure-authentication/google/index.md +++ b/docs/sources/setup-grafana/configure-security/configure-authentication/google/index.md @@ -53,3 +53,15 @@ You may allow users to sign-up via Google authentication by setting the `allow_sign_up` option to `true`. When this option is set to `true`, any user successfully authenticating via Google authentication will be automatically signed up. + +### Configure refresh token + +> Available in Grafana v9.3 and later versions. + +> **Note:** This feature is behind the `accessTokenExpirationCheck` feature toggle. + +When a user logs in using an OAuth provider, Grafana verifies that the access token has not expired. When an access token expires, Grafana uses the provided refresh token (if any exists) to obtain a new access token. + +Grafana uses a refresh token to obtain a new access token without requiring the user to log in again. If a refresh token doesn't exist, Grafana logs the user out of the system after the access token has expired. + +By default, Grafana includes the `access_type=offline` parameter in the authorization request to request a refresh token. diff --git a/docs/sources/setup-grafana/configure-security/configure-authentication/keycloak/index.md b/docs/sources/setup-grafana/configure-security/configure-authentication/keycloak/index.md index dac2111f587..c5129835cc1 100644 --- a/docs/sources/setup-grafana/configure-security/configure-authentication/keycloak/index.md +++ b/docs/sources/setup-grafana/configure-security/configure-authentication/keycloak/index.md @@ -141,3 +141,15 @@ Grafana also assigns the user the `Admin` role of the default organization. role_attribute_path = contains(roles[*], 'grafanaadmin') && 'GrafanaAdmin' || contains(roles[*], 'admin') && 'Admin' || contains(roles[*], 'editor') && 'Editor' || 'Viewer' allow_assign_grafana_admin = true ``` + +### Configure refresh token + +> Available in Grafana v9.3 and later versions. + +> **Note:** This feature is behind the `accessTokenExpirationCheck` feature toggle. + +When a user logs in using an OAuth provider, Grafana verifies that the access token has not expired. When an access token expires, Grafana uses the provided refresh token (if any exists) to obtain a new access token. + +Grafana uses a refresh token to obtain a new access token without requiring the user to log in again. If a refresh token doesn't exist, Grafana logs the user out of the system after the access token has expired. + +To enable a refresh token for Keycloak, extend the `scopes` in `[auth.generic_oauth]` with `offline_access`. diff --git a/docs/sources/setup-grafana/configure-security/configure-authentication/okta/index.md b/docs/sources/setup-grafana/configure-security/configure-authentication/okta/index.md index c4eac856b34..86c2858aae3 100644 --- a/docs/sources/setup-grafana/configure-security/configure-authentication/okta/index.md +++ b/docs/sources/setup-grafana/configure-security/configure-authentication/okta/index.md @@ -54,6 +54,19 @@ allowed_groups = role_attribute_path = ``` +### Configure refresh token + +> Available in Grafana v9.3 and later versions. + +> **Note:** This feature is behind the `accessTokenExpirationCheck` feature toggle. + +When a user logs in using an OAuth provider, Grafana verifies that the access token has not expired. When an access token expires, Grafana uses the provided refresh token (if any exists) to obtain a new access token. + +Grafana uses a refresh token to obtain a new access token without requiring the user to log in again. If a refresh token doesn't exist, Grafana logs the user out of the system after the access token has expired. + +1. To enable the `Refresh Token`, grant type in the `General Settings` section. +1. Extend the `scopes` in `[auth.okta]` with `offline_access`. + ### Configure allowed groups and domains To limit access to authenticated users that are members of one or more groups, set `allowed_groups` diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index c115b43a80a..c4b9823408c 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -80,5 +80,6 @@ export interface FeatureToggles { datasourceLogger?: boolean; accessControlOnCall?: boolean; nestedFolders?: boolean; + accessTokenExpirationCheck?: boolean; elasticsearchBackendMigration?: boolean; } diff --git a/pkg/api/common_test.go b/pkg/api/common_test.go index 8ca9c240b8e..4f91f5621e6 100644 --- a/pkg/api/common_test.go +++ b/pkg/api/common_test.go @@ -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 } diff --git a/pkg/api/login_oauth.go b/pkg/api/login_oauth.go index 68566f702d8..88a465d7fdb 100644 --- a/pkg/api/login_oauth.go +++ b/pkg/api/login_oauth.go @@ -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() diff --git a/pkg/middleware/middleware_test.go b/pkg/middleware/middleware_test.go index 21dc23540f9..28839e6198c 100644 --- a/pkg/middleware/middleware_test.go +++ b/pkg/middleware/middleware_test.go @@ -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 { diff --git a/pkg/services/contexthandler/auth_proxy_test.go b/pkg/services/contexthandler/auth_proxy_test.go index b62e6e5d97c..9e6a629ab2b 100644 --- a/pkg/services/contexthandler/auth_proxy_test.go +++ b/pkg/services/contexthandler/auth_proxy_test.go @@ -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 { diff --git a/pkg/services/contexthandler/contexthandler.go b/pkg/services/contexthandler/contexthandler.go index a19c8c2d7fb..0179a8bccf0 100644 --- a/pkg/services/contexthandler/contexthandler.go +++ b/pkg/services/contexthandler/contexthandler.go @@ -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 } } } diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index 39f308edf15..62352cfe6a3 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -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", diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index 325fa48559a..2441a1ab0fe 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -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"