From fe49ae05c0fd1389e577119925ada87e8fbbf98b Mon Sep 17 00:00:00 2001 From: Victor Cinaglia Date: Wed, 17 Dec 2025 09:03:29 -0300 Subject: [PATCH 01/22] Auth: Disable login prompt option for Google OAuth when "use_refresh_token" is enabled (#115367) * Auth: Google OAuth consent prompt takes precedence when use_refresh_token is true * Auth: Disable login prompt option for Google OAuth when use_refresh_token is true * yarn run prettier:check --write * feedback: validate login prompt when use_refresh_token is true --- pkg/login/social/connectors/google_oauth.go | 10 +- .../social/connectors/google_oauth_test.go | 142 +++++++++++++++++- pkg/login/social/connectors/social_base.go | 6 +- .../features/auth-config/FieldRenderer.tsx | 28 +++- public/app/features/auth-config/fields.tsx | 37 +++-- public/app/features/auth-config/types.ts | 17 ++- public/locales/en-US/grafana.json | 2 + 7 files changed, 222 insertions(+), 20 deletions(-) diff --git a/pkg/login/social/connectors/google_oauth.go b/pkg/login/social/connectors/google_oauth.go index 81c7cd31f9d..b94f0514879 100644 --- a/pkg/login/social/connectors/google_oauth.go +++ b/pkg/login/social/connectors/google_oauth.go @@ -81,7 +81,15 @@ func (s *SocialGoogle) Validate(ctx context.Context, newSettings ssoModels.SSOSe return validation.Validate(info, requester, validation.MustBeEmptyValidator(info.AuthUrl, "Auth URL"), validation.MustBeEmptyValidator(info.TokenUrl, "Token URL"), - validation.MustBeEmptyValidator(info.ApiUrl, "API URL")) + validation.MustBeEmptyValidator(info.ApiUrl, "API URL"), + loginPromptValidator) +} + +func loginPromptValidator(info *social.OAuthInfo, requester identity.Requester) error { + if info.UseRefreshToken && !slices.Contains([]string{"", "consent"}, info.LoginPrompt) { + return ssosettings.ErrInvalidOAuthConfig("If provided, login_prompt must be set to consent when use_refresh_token is enabled.") + } + return nil } func (s *SocialGoogle) Reload(ctx context.Context, settings ssoModels.SSOSettings) error { diff --git a/pkg/login/social/connectors/google_oauth_test.go b/pkg/login/social/connectors/google_oauth_test.go index d330c39d78d..620c448013b 100644 --- a/pkg/login/social/connectors/google_oauth_test.go +++ b/pkg/login/social/connectors/google_oauth_test.go @@ -9,6 +9,7 @@ import ( "fmt" "net/http" "net/http/httptest" + "net/url" "testing" "time" @@ -18,6 +19,7 @@ import ( "github.com/stretchr/testify/require" "golang.org/x/oauth2" + "github.com/grafana/grafana/pkg/apimachinery/errutil" "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/login/social" "github.com/grafana/grafana/pkg/services/featuremgmt" @@ -871,6 +873,39 @@ func TestSocialGoogle_Validate(t *testing.T) { }, wantErr: ssosettings.ErrBaseInvalidOAuthConfig, }, + { + name: "fails if use_refresh_token is enabled and login prompt is neither empty or 'consent'", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "use_refresh_token": "true", + "login_prompt": "login", + }, + }, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + }, + { + name: "succeeds if use_refresh_token is enabled and login prompt is empty", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "use_refresh_token": "true", + "login_prompt": "", + }, + }, + wantErr: nil, + }, + { + name: "succeeds if use_refresh_token is enabled and login prompt is consent", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "use_refresh_token": "true", + "login_prompt": "consent", + }, + }, + wantErr: nil, + }, } for _, tc := range testCases { @@ -886,7 +921,13 @@ func TestSocialGoogle_Validate(t *testing.T) { require.ErrorIs(t, err, tc.wantErr) return } - require.NoError(t, err) + + if err != nil { + var e errutil.Error + require.True(t, errors.As(err, &e)) + require.NoError(t, e, "expected no error, got %v", e.PublicMessage) + return + } }) } } @@ -1024,3 +1065,102 @@ func TestIsHDAllowed(t *testing.T) { }) } } + +func TestSocialGoogle_AuthCodeURL(t *testing.T) { + testCases := []struct { + name string + info *social.OAuthInfo + opts []oauth2.AuthCodeOption + state string + wantURL *url.URL + }{ + { + name: "should return the correct auth code URL", + info: &social.OAuthInfo{ + ClientId: "client-id", + ClientSecret: "client-secret", + AuthUrl: "https://example.com/auth", + LoginPrompt: "login", + Scopes: []string{"openid", "email", "profile"}, + }, + state: "test-state", + opts: []oauth2.AuthCodeOption{ + oauth2.SetAuthURLParam("extra_param", "extra_value"), + }, + wantURL: &url.URL{ + Scheme: "https", + Host: "example.com", + Path: "/auth", + RawQuery: url.Values{ + "state": {"test-state"}, + "prompt": {"login"}, + "response_type": {"code"}, + "client_id": {"client-id"}, + "redirect_uri": {"/login/google"}, + "scope": {"openid email profile"}, + "extra_param": {"extra_value"}, + }.Encode(), + }, + }, + { + name: "should add access type offline and approval force if use refresh token is enabled", + info: &social.OAuthInfo{ + ClientId: "client-id", + ClientSecret: "client-secret", + AuthUrl: "https://example.com/auth", + Scopes: []string{"openid", "email", "profile"}, + UseRefreshToken: true, + }, + state: "test-state", + wantURL: &url.URL{ + Scheme: "https", + Host: "example.com", + Path: "/auth", + RawQuery: url.Values{ + "state": {"test-state"}, + "prompt": {"consent"}, + "response_type": {"code"}, + "client_id": {"client-id"}, + "redirect_uri": {"/login/google"}, + "scope": {"openid email profile"}, + "access_type": {"offline"}, + }.Encode(), + }, + }, + { + name: "should override configured login prompt if use refresh token is enabled", + info: &social.OAuthInfo{ + ClientId: "client-id", + ClientSecret: "client-secret", + AuthUrl: "https://example.com/auth", + Scopes: []string{"openid", "email", "profile"}, + UseRefreshToken: true, + }, + state: "test-state", + wantURL: &url.URL{ + Scheme: "https", + Host: "example.com", + Path: "/auth", + RawQuery: url.Values{ + "state": {"test-state"}, + "prompt": {"consent"}, + "response_type": {"code"}, + "client_id": {"client-id"}, + "redirect_uri": {"/login/google"}, + "scope": {"openid email profile"}, + "access_type": {"offline"}, + }.Encode(), + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + s := NewGoogleProvider(tc.info, &setting.Cfg{}, nil, ssosettingstests.NewFakeService(), featuremgmt.WithFeatures()) + gotURL := s.AuthCodeURL(tc.state, tc.opts...) + parsedURL, err := url.Parse(gotURL) + require.NoError(t, err) + require.EqualValues(t, tc.wantURL, parsedURL) + }) + } +} diff --git a/pkg/login/social/connectors/social_base.go b/pkg/login/social/connectors/social_base.go index 0db4d81baeb..3ecbf8e334e 100644 --- a/pkg/login/social/connectors/social_base.go +++ b/pkg/login/social/connectors/social_base.go @@ -91,7 +91,11 @@ func (s *SocialBase) AuthCodeURL(state string, opts ...oauth2.AuthCodeOption) st func (s *SocialBase) getAuthCodeURL(state string, opts ...oauth2.AuthCodeOption) string { if s.info.LoginPrompt != "" { promptOpt := oauth2.SetAuthURLParam("prompt", s.info.LoginPrompt) - opts = append(opts, promptOpt) + + // Prepend the prompt option to the opts slice to ensure it is applied last. + // This is necessary in case the caller provides an option that overrides the prompt, + // such as `oauth2.ApprovalForce`. + opts = append([]oauth2.AuthCodeOption{promptOpt}, opts...) } return s.Config.AuthCodeURL(state, opts...) diff --git a/public/app/features/auth-config/FieldRenderer.tsx b/public/app/features/auth-config/FieldRenderer.tsx index 1d80f835fb0..687079f1281 100644 --- a/public/app/features/auth-config/FieldRenderer.tsx +++ b/public/app/features/auth-config/FieldRenderer.tsx @@ -35,17 +35,23 @@ export const FieldRenderer = ({ const [isSecretConfigured, setIsSecretConfigured] = useState(secretConfigured); const isDependantField = typeof field !== 'string'; const name = isDependantField ? field.name : field; - const parentValue = isDependantField ? watch(field.dependsOn) : null; + const parentValue = isDependantField && field.dependsOn ? watch(field.dependsOn) : null; const fieldData = fieldMap(provider)[name]; const theme = useTheme2(); + + // Handle disabledWhen configuration + const disabledWhen = isDependantField ? field.disabledWhen : undefined; + const disabledWhenValue = disabledWhen ? watch(disabledWhen.field) : undefined; + const isDisabled = disabledWhen ? disabledWhenValue === disabledWhen.is : false; + // Unregister a field that depends on a toggle to clear its data useEffect(() => { - if (isDependantField) { + if (isDependantField && field.dependsOn) { if (!parentValue) { unregister(name); } } - }, [unregister, name, parentValue, isDependantField]); + }, [unregister, name, parentValue, isDependantField, field]); const isNotEmptySelectableValueArray = ( current: string | boolean | Record | Array> | undefined @@ -64,6 +70,13 @@ export const FieldRenderer = ({ // eslint-disable-next-line react-hooks/exhaustive-deps }, []); + // Set the value when the field is disabled + useEffect(() => { + if (isDisabled && disabledWhen?.disabledValue) { + setValue(name, disabledWhen.disabledValue.value); + } + }, [isDisabled, disabledWhen?.disabledValue, name, setValue]); + if (!field) { console.log('missing field:', name); return null; @@ -74,12 +87,12 @@ export const FieldRenderer = ({ } // Dependant field means the field depends on another field's value and shouldn't be rendered if the parent field is false - if (isDependantField) { - const parentValue = watch(field.dependsOn); + if (isDependantField && field.dependsOn) { if (!parentValue) { return null; } } + const fieldProps = { label: fieldData.label, required: !!fieldData.validation?.required, @@ -131,10 +144,10 @@ export const FieldRenderer = ({ rules={fieldData.validation} name={name} control={control} - render={({ field: { ref, onChange, ...fieldProps }, fieldState: { invalid } }) => { + render={({ field: { ref, onChange, ...controllerFieldProps }, fieldState: { invalid } }) => { return (