diff --git a/conf/defaults.ini b/conf/defaults.ini index f21b0ea8ee9..2d3f108f5bd 100644 --- a/conf/defaults.ini +++ b/conf/defaults.ini @@ -679,6 +679,7 @@ token_url = https://oauth2.googleapis.com/token api_url = https://openidconnect.googleapis.com/v1/userinfo signout_redirect_url = allowed_domains = +validate_hd = true hosted_domain = allowed_groups = role_attribute_path = diff --git a/conf/sample.ini b/conf/sample.ini index e153c17c2ac..876585d097e 100644 --- a/conf/sample.ini +++ b/conf/sample.ini @@ -643,6 +643,7 @@ ;api_url = https://openidconnect.googleapis.com/v1/userinfo ;signout_redirect_url = ;allowed_domains = +;validate_hd = ;hosted_domain = ;allowed_groups = ;role_attribute_path = 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 7d0fe701c14..38c185d6064 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 @@ -111,6 +111,14 @@ automatically signed up. You may specify a domain to be passed as `hd` query parameter accepted by Google's OAuth 2.0 authentication API. Refer to Google's OAuth [documentation](https://developers.google.com/identity/openid-connect/openid-connect#hd-param). +{{% admonition type="note" %}} +Since Grafana 10.3.0, the `hd` parameter retrieved from Google ID token is also used to determine the user's hosted domain. The Google Oauth `allowed_domains` configuration option is used to restrict access to users from a specific domain. If the `allowed_domains` configuration option is set, the `hd` parameter from the Google ID token must match the `allowed_domains` configuration option. If the `hd` parameter from the Google ID token does not match the `allowed_domains` configuration option, the user is denied access. + +When an account does not belong to a google workspace, the hd claim will not be available. + +This validation is enabled by default. To disable this validation, set the `validate_hd` configuration option to `false`. The `allowed_domains` configuration option will use the email claim to validate the domain. +{{% /admonition %}} + #### PKCE IETF's [RFC 7636](https://datatracker.ietf.org/doc/html/rfc7636) diff --git a/pkg/login/social/connectors/google_oauth.go b/pkg/login/social/connectors/google_oauth.go index a4404004afa..84c6d9f96f4 100644 --- a/pkg/login/social/connectors/google_oauth.go +++ b/pkg/login/social/connectors/google_oauth.go @@ -24,13 +24,16 @@ const ( legacyAPIURL = "https://www.googleapis.com/oauth2/v1/userinfo" googleIAMGroupsEndpoint = "https://content-cloudidentity.googleapis.com/v1/groups/-/memberships:searchDirectGroups" googleIAMScope = "https://www.googleapis.com/auth/cloud-identity.groups.readonly" + validateHDKey = "validate_hd" ) var _ social.SocialConnector = (*SocialGoogle)(nil) var _ ssosettings.Reloadable = (*SocialGoogle)(nil) +var ExtraGoogleSettingKeys = []string{validateHDKey} type SocialGoogle struct { *SocialBase + validateHD bool } type googleUserData struct { @@ -45,6 +48,7 @@ type googleUserData struct { func NewGoogleProvider(info *social.OAuthInfo, cfg *setting.Cfg, ssoSettings ssosettings.Service, features featuremgmt.FeatureToggles) *SocialGoogle { provider := &SocialGoogle{ SocialBase: newSocialBase(social.GoogleProviderName, info, features, cfg), + validateHD: MustBool(info.Extra[validateHDKey], true), } if strings.HasPrefix(info.ApiUrl, legacyAPIURL) { @@ -89,6 +93,7 @@ func (s *SocialGoogle) Reload(ctx context.Context, settings ssoModels.SSOSetting defer s.reloadMutex.Unlock() s.updateInfo(social.GoogleProviderName, newInfo) + s.validateHD = MustBool(newInfo.Extra[validateHDKey], false) return nil } @@ -117,7 +122,7 @@ func (s *SocialGoogle) UserInfo(ctx context.Context, client *http.Client, token return nil, fmt.Errorf("user email is not verified") } - if err := s.isHDAllowed(data.HD); err != nil { + if err := s.isHDAllowed(data.HD, info); err != nil { return nil, err } @@ -163,6 +168,7 @@ type googleAPIData struct { Name string `json:"name"` Email string `json:"email"` EmailVerified bool `json:"verified_email"` + HD string `json:"hd"` } func (s *SocialGoogle) extractFromAPI(ctx context.Context, client *http.Client) (*googleUserData, error) { @@ -184,6 +190,7 @@ func (s *SocialGoogle) extractFromAPI(ctx context.Context, client *http.Client) Name: data.Name, Email: data.Email, EmailVerified: data.EmailVerified, + HD: data.HD, rawJSON: response.Body, }, nil } @@ -297,12 +304,16 @@ func (s *SocialGoogle) getGroupsPage(ctx context.Context, client *http.Client, u return &data, nil } -func (s *SocialGoogle) isHDAllowed(hd string) error { - if len(s.info.AllowedDomains) == 0 { +func (s *SocialGoogle) isHDAllowed(hd string, info *social.OAuthInfo) error { + if s.validateHD { return nil } - for _, allowedDomain := range s.info.AllowedDomains { + if len(info.AllowedDomains) == 0 { + return nil + } + + for _, allowedDomain := range info.AllowedDomains { if hd == allowedDomain { return nil } diff --git a/pkg/login/social/connectors/google_oauth_test.go b/pkg/login/social/connectors/google_oauth_test.go index 7fe0d8b0244..d822d4d7b88 100644 --- a/pkg/login/social/connectors/google_oauth_test.go +++ b/pkg/login/social/connectors/google_oauth_test.go @@ -897,6 +897,7 @@ func TestIsHDAllowed(t *testing.T) { email string allowedDomains []string expectedErrorMessage string + validateHD bool }{ { name: "should not fail if no allowed domains are set", @@ -916,6 +917,12 @@ func TestIsHDAllowed(t *testing.T) { allowedDomains: []string{"grafana.com", "example.com"}, expectedErrorMessage: "the hd claim found in the ID token is not present in the allowed domains", }, + { + name: "should not fail if the HD validation is disabled and the email not being from an allowed domain", + email: "mycompany.com", + allowedDomains: []string{"grafana.com", "example.com"}, + validateHD: true, + }, } for _, tc := range testCases { @@ -923,7 +930,8 @@ func TestIsHDAllowed(t *testing.T) { info := &social.OAuthInfo{} info.AllowedDomains = tc.allowedDomains s := NewGoogleProvider(info, &setting.Cfg{}, &ssosettingstests.MockService{}, featuremgmt.WithFeatures()) - err := s.isHDAllowed(tc.email) + s.validateHD = tc.validateHD + err := s.isHDAllowed(tc.email, info) if tc.expectedErrorMessage != "" { require.Error(t, err) diff --git a/pkg/services/ssosettings/strategies/oauth_strategy.go b/pkg/services/ssosettings/strategies/oauth_strategy.go index 4dbbfddfbe0..7f67ad9c8e3 100644 --- a/pkg/services/ssosettings/strategies/oauth_strategy.go +++ b/pkg/services/ssosettings/strategies/oauth_strategy.go @@ -19,6 +19,7 @@ var extraKeysByProvider = map[string][]string{ social.AzureADProviderName: connectors.ExtraAzureADSettingKeys, social.GenericOAuthProviderName: connectors.ExtraGenericOAuthSettingKeys, social.GitHubProviderName: connectors.ExtraGithubSettingKeys, + social.GoogleProviderName: connectors.ExtraGoogleSettingKeys, social.GrafanaComProviderName: connectors.ExtraGrafanaComSettingKeys, social.GrafanaNetProviderName: connectors.ExtraGrafanaComSettingKeys, } diff --git a/pkg/services/ssosettings/strategies/oauth_strategy_test.go b/pkg/services/ssosettings/strategies/oauth_strategy_test.go index b41e14e1b3f..3cf7031d7dc 100644 --- a/pkg/services/ssosettings/strategies/oauth_strategy_test.go +++ b/pkg/services/ssosettings/strategies/oauth_strategy_test.go @@ -7,6 +7,7 @@ import ( "github.com/stretchr/testify/require" "gopkg.in/ini.v1" + "github.com/grafana/grafana/pkg/login/social" "github.com/grafana/grafana/pkg/setting" ) @@ -129,6 +130,9 @@ func TestGetProviderConfig_ExtraFields(t *testing.T) { [auth.grafana_com] enabled = true allowed_organizations = org1, org2 + + [auth.google] + validate_hd = true ` iniFile, err := ini.Load([]byte(iniWithExtraFields)) @@ -139,24 +143,24 @@ func TestGetProviderConfig_ExtraFields(t *testing.T) { strategy := NewOAuthStrategy(cfg) - t.Run("azuread", func(t *testing.T) { - result, err := strategy.GetProviderConfig(context.Background(), "azuread") + t.Run(social.AzureADProviderName, func(t *testing.T) { + result, err := strategy.GetProviderConfig(context.Background(), social.AzureADProviderName) require.NoError(t, err) require.Equal(t, "true", result["force_use_graph_api"]) require.Equal(t, "org1, org2", result["allowed_organizations"]) }) - t.Run("github", func(t *testing.T) { - result, err := strategy.GetProviderConfig(context.Background(), "github") + t.Run(social.GitHubProviderName, func(t *testing.T) { + result, err := strategy.GetProviderConfig(context.Background(), social.GitHubProviderName) require.NoError(t, err) require.Equal(t, "first, second", result["team_ids"]) require.Equal(t, "org1, org2", result["allowed_organizations"]) }) - t.Run("generic_oauth", func(t *testing.T) { - result, err := strategy.GetProviderConfig(context.Background(), "generic_oauth") + t.Run(social.GenericOAuthProviderName, func(t *testing.T) { + result, err := strategy.GetProviderConfig(context.Background(), social.GenericOAuthProviderName) require.NoError(t, err) require.Equal(t, "first, second", result["team_ids"]) @@ -166,12 +170,19 @@ func TestGetProviderConfig_ExtraFields(t *testing.T) { require.Equal(t, "id_token", result["id_token_attribute_name"]) }) - t.Run("grafana_com", func(t *testing.T) { - result, err := strategy.GetProviderConfig(context.Background(), "grafana_com") + t.Run(social.GrafanaComProviderName, func(t *testing.T) { + result, err := strategy.GetProviderConfig(context.Background(), social.GrafanaComProviderName) require.NoError(t, err) require.Equal(t, "org1, org2", result["allowed_organizations"]) }) + + t.Run(social.GoogleProviderName, func(t *testing.T) { + result, err := strategy.GetProviderConfig(context.Background(), social.GoogleProviderName) + require.NoError(t, err) + + require.Equal(t, "true", result["validate_hd"]) + }) } // TestGetProviderConfig_GrafanaComGrafanaNet tests that the connector is setup using the correct section and it supports