From 8ad9b65500f06ee54f162c30f06bb13e248edabe Mon Sep 17 00:00:00 2001 From: Agni Bhattacharyya <34892434+PyAgni@users.noreply.github.com> Date: Fri, 9 Aug 2024 20:50:54 +0530 Subject: [PATCH] Auth: Skip email extraction when api url is not present (#91699) * Auth: Skip email extraction when api url is not present * fix lint: reduce cyclomatic complexity (cherry picked from commit be32630de5d3e81cb299a0c97946b3258d8ba9bf) --- pkg/login/social/connectors/generic_oauth.go | 6 +++- .../social/connectors/generic_oauth_test.go | 35 +++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/pkg/login/social/connectors/generic_oauth.go b/pkg/login/social/connectors/generic_oauth.go index 84a8e00d3fd..5d85302dd40 100644 --- a/pkg/login/social/connectors/generic_oauth.go +++ b/pkg/login/social/connectors/generic_oauth.go @@ -305,7 +305,7 @@ func (s *SocialGenericOAuth) UserInfo(ctx context.Context, client *http.Client, s.log.Debug("AllowAssignGrafanaAdmin and skipOrgRoleSync are both set, Grafana Admin role will not be synced, consider setting one or the other") } - if userInfo.Email == "" { + if s.canFetchPrivateEmail(userInfo) { var err error userInfo.Email, err = s.fetchPrivateEmail(ctx, client) if err != nil { @@ -335,6 +335,10 @@ func (s *SocialGenericOAuth) UserInfo(ctx context.Context, client *http.Client, return userInfo, nil } +func (s *SocialGenericOAuth) canFetchPrivateEmail(userinfo *social.BasicUserInfo) bool { + return s.info.ApiUrl != "" && userinfo.Email == "" +} + func (s *SocialGenericOAuth) extractFromToken(token *oauth2.Token) *UserInfoJson { s.log.Debug("Extracting user info from OAuth token") diff --git a/pkg/login/social/connectors/generic_oauth_test.go b/pkg/login/social/connectors/generic_oauth_test.go index 5bcd8204d92..36f9d13fd91 100644 --- a/pkg/login/social/connectors/generic_oauth_test.go +++ b/pkg/login/social/connectors/generic_oauth_test.go @@ -499,6 +499,41 @@ func TestUserInfoSearchesForEmailAndOrgRoles(t *testing.T) { require.Equal(t, tc.ExpectedGrafanaAdmin, actualResult.IsGrafanaAdmin) }) } + + t.Run("Generic OAuth with empty API URL shouldn't call fetchPrivateEmail function", func(t *testing.T) { + orgSvc := &orgtest.FakeOrgService{ExpectedOrgs: []*org.OrgDTO{{ID: 4, Name: "org_dev"}, {ID: 5, Name: "org_engineering"}}} + orgRoleMapper := ProvideOrgRoleMapper(cfg, orgSvc) + provider := NewGenericOAuthProvider(&social.OAuthInfo{ + EmailAttributePath: "email", + }, cfg, + orgRoleMapper, + &ssosettingstests.MockService{}, + featuremgmt.WithFeatures()) + + body, err := json.Marshal(map[string]any{"info": map[string]any{"roles": []string{"engineering", "SRE"}}}) + require.NoError(t, err) + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + w.Header().Set("Content-Type", "application/json") + _, err = w.Write(body) + require.NoError(t, err) + })) + + provider.info.ApiUrl = "" + staticToken := oauth2.Token{ + AccessToken: "", + TokenType: "", + RefreshToken: "", + Expiry: time.Now(), + } + + token := staticToken.WithExtra(map[string]any{ + "id_token": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJyb2xlIjoiQWRtaW4iLCJlbWFpbCI6IiJ9.hQPKYTPXyEYAD_cS6uxBDJcG8ucLePR3thBBQST6tQs", + }) + actualResult, err := provider.UserInfo(context.Background(), ts.Client(), token) + require.NoError(t, err) + require.Equal(t, "", actualResult.Email) + }) } func TestUserInfoSearchesForLogin(t *testing.T) {