From 164873591b2f5224cc3777a0ab6406f76083167d Mon Sep 17 00:00:00 2001 From: Victor Cinaglia Date: Wed, 19 Nov 2025 15:21:40 -0300 Subject: [PATCH] IAM: Optionally make refresh tokens required if use_refresh_token is enabled (#114174) * OAuth: Optionally make refresh tokens required if use_refresh_token is enabled * make linter happy * feedback: log missing refresh token during token refresh * feedback: tweak wording in the message & change level --- .../src/types/featureToggles.gen.ts | 4 ++ pkg/services/authn/clients/oauth.go | 11 ++++ pkg/services/authn/clients/oauth_test.go | 57 +++++++++++++++++++ pkg/services/featuremgmt/registry.go | 8 +++ pkg/services/featuremgmt/toggles_gen.csv | 1 + pkg/services/featuremgmt/toggles_gen.go | 4 ++ pkg/services/featuremgmt/toggles_gen.json | 14 +++++ pkg/services/oauthtoken/oauth_token.go | 4 ++ 8 files changed, 103 insertions(+) diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index edfb1056977..b9afd13cec0 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -477,6 +477,10 @@ export interface FeatureToggles { */ oauthRequireSubClaim?: boolean; /** + * Require that refresh tokens are present in oauth tokens. + */ + refreshTokenRequired?: boolean; + /** * Enables filters and group by variables on all new dashboards. Variables are added only if default data source supports filtering. */ newDashboardWithFiltersAndGroupBy?: boolean; diff --git a/pkg/services/authn/clients/oauth.go b/pkg/services/authn/clients/oauth.go index b65202b71d4..f7798cc365f 100644 --- a/pkg/services/authn/clients/oauth.go +++ b/pkg/services/authn/clients/oauth.go @@ -55,6 +55,8 @@ var ( errOAuthTokenExchange = errutil.Internal("auth.oauth.token.exchange", errutil.WithPublicMessage("Failed to get token from provider")) errOAuthUserInfo = errutil.Internal("auth.oauth.userinfo.error") + errOAuthMissingRefreshToken = errutil.Unauthorized("auth.oauth.token.refresh-token.missing", errutil.WithPublicMessage("Provider did not return a refresh token")) + errOAuthMissingRequiredEmail = errutil.Unauthorized("auth.oauth.email.missing", errutil.WithPublicMessage("Provider didn't return an email address")) errOAuthEmailNotAllowed = errutil.Unauthorized("auth.oauth.email.not-allowed", errutil.WithPublicMessage("Required email domain not fulfilled")) ) @@ -166,6 +168,15 @@ func (c *OAuth) Authenticate(ctx context.Context, r *authn.Request) (*authn.Iden } token.TokenType = "Bearer" + if oauthCfg.UseRefreshToken && token.RefreshToken == "" { + c.log.FromContext(ctx).Warn("No refresh token available with use_refresh_token enabled", "authmodule", c.moduleName) + + //nolint:staticcheck // not yet migrated to OpenFeature + if c.features.IsEnabledGlobally(featuremgmt.FlagRefreshTokenRequired) { + return nil, errOAuthMissingRefreshToken.Errorf("provider did not return a refresh token") + } + } + userInfo, err := connector.UserInfo(ctx, connector.Client(clientCtx, token), token) if err != nil { var sErr *connectors.SocialError diff --git a/pkg/services/authn/clients/oauth_test.go b/pkg/services/authn/clients/oauth_test.go index b6a9a07c488..324b956bd36 100644 --- a/pkg/services/authn/clients/oauth_test.go +++ b/pkg/services/authn/clients/oauth_test.go @@ -265,6 +265,63 @@ func TestOAuth_Authenticate(t *testing.T) { }, }, }, + { + desc: "should return error when no refresh token is available and feature toggle is enabled", + req: &authn.Request{ + HTTPRequest: &http.Request{ + Header: map[string][]string{}, + URL: mustParseURL("http://grafana.com/?state=some-state"), + }, + }, + oauthCfg: &social.OAuthInfo{UsePKCE: true, Enabled: true, UseRefreshToken: true}, + features: []any{featuremgmt.FlagRefreshTokenRequired}, + allowInsecureTakeover: true, + addStateCookie: true, + stateCookieValue: "some-state", + addPKCECookie: true, + pkceCookieValue: "some-pkce-value", + isEmailAllowed: true, + expectedErr: errOAuthMissingRefreshToken, + }, + { + desc: "should return identity when no refresh token is available and feature toggle is disabled", + req: &authn.Request{ + HTTPRequest: &http.Request{ + Header: map[string][]string{}, + URL: mustParseURL("http://grafana.com/?state=some-state"), + }, + }, + oauthCfg: &social.OAuthInfo{UsePKCE: true, Enabled: true, UseRefreshToken: true}, + addStateCookie: true, + stateCookieValue: "some-state", + addPKCECookie: true, + pkceCookieValue: "some-pkce-value", + isEmailAllowed: true, + userInfo: &social.BasicUserInfo{ + Id: "123", + Name: "name", + Email: "some@email.com", + Role: "Admin", + Groups: []string{"grp1", "grp2"}, + }, + expectedIdentity: &authn.Identity{ + Email: "some@email.com", + AuthenticatedBy: login.AzureADAuthModule, + AuthID: "123", + Name: "name", + Groups: []string{"grp1", "grp2"}, + OAuthToken: &oauth2.Token{}, + OrgRoles: map[int64]org.RoleType{1: org.RoleAdmin}, + ClientParams: authn.ClientParams{ + SyncUser: true, + SyncTeams: true, + AllowSignUp: true, + FetchSyncedUser: true, + SyncOrgRoles: true, + LookUpParams: login.UserLookupParams{}, + }, + }, + }, } for _, tt := range tests { diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index b7362496e38..68e88830be4 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -817,6 +817,14 @@ var ( HideFromDocs: true, HideFromAdminPage: true, }, + { + Name: "refreshTokenRequired", + Description: "Require that refresh tokens are present in oauth tokens.", + Stage: FeatureStageExperimental, + Owner: identityAccessTeam, + HideFromDocs: true, + HideFromAdminPage: true, + }, { Name: "newDashboardWithFiltersAndGroupBy", Description: "Enables filters and group by variables on all new dashboards. Variables are added only if default data source supports filtering.", diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index 2d2688c285c..e0f2bec13bc 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -107,6 +107,7 @@ kubernetesAggregatorCapTokenAuth,experimental,@grafana/grafana-app-platform-squa groupByVariable,experimental,@grafana/dashboards-squad,false,false,false scopeFilters,experimental,@grafana/dashboards-squad,false,false,false oauthRequireSubClaim,experimental,@grafana/identity-access-team,false,false,false +refreshTokenRequired,experimental,@grafana/identity-access-team,false,false,false newDashboardWithFiltersAndGroupBy,experimental,@grafana/dashboards-squad,false,false,false cloudWatchNewLabelParsing,GA,@grafana/aws-datasources,false,false,false disableNumericMetricsSortingInExpressions,experimental,@grafana/oss-big-tent,false,true,false diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index f5792d32d88..9fb1c573406 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -439,6 +439,10 @@ const ( // Require that sub claims is present in oauth tokens. FlagOauthRequireSubClaim = "oauthRequireSubClaim" + // FlagRefreshTokenRequired + // Require that refresh tokens are present in oauth tokens. + FlagRefreshTokenRequired = "refreshTokenRequired" + // FlagNewDashboardWithFiltersAndGroupBy // Enables filters and group by variables on all new dashboards. Variables are added only if default data source supports filtering. FlagNewDashboardWithFiltersAndGroupBy = "newDashboardWithFiltersAndGroupBy" diff --git a/pkg/services/featuremgmt/toggles_gen.json b/pkg/services/featuremgmt/toggles_gen.json index 0fc14a616c5..daf3c48c40c 100644 --- a/pkg/services/featuremgmt/toggles_gen.json +++ b/pkg/services/featuremgmt/toggles_gen.json @@ -3543,6 +3543,20 @@ "hideFromAdminPage": true } }, + { + "metadata": { + "name": "refreshTokenRequired", + "resourceVersion": "1763561990273", + "creationTimestamp": "2025-11-19T14:19:50Z" + }, + "spec": { + "description": "Require that refresh tokens are present in oauth tokens.", + "stage": "experimental", + "codeowner": "@grafana/identity-access-team", + "hideFromAdminPage": true, + "hideFromDocs": true + } + }, { "metadata": { "name": "regressionTransformation", diff --git a/pkg/services/oauthtoken/oauth_token.go b/pkg/services/oauthtoken/oauth_token.go index ee0de1007b2..0efe5e553f3 100644 --- a/pkg/services/oauthtoken/oauth_token.go +++ b/pkg/services/oauthtoken/oauth_token.go @@ -473,6 +473,10 @@ func (o *Service) tryGetOrRefreshOAuthToken(ctx context.Context, persistedToken ) } + if token.RefreshToken == "" { + ctxLogger.Warn("Refresh token is missing after token refresh", "authmodule", tokenRefreshMetadata.AuthModule) + } + //nolint:staticcheck // not yet migrated to OpenFeature if !o.features.IsEnabledGlobally(featuremgmt.FlagImprovedExternalSessionHandling) { updateAuthCommand := &login.UpdateAuthInfoCommand{