From e3314f04e4813ae6a86e14a09c475e49a018840d Mon Sep 17 00:00:00 2001 From: Mihai Doarna Date: Thu, 7 Mar 2024 10:32:55 +0200 Subject: [PATCH] Auth: perform read locking on every exported func from social providers (#83960) perform read locking on every exported func from social providers --- pkg/login/social/connectors/azuread_oauth.go | 24 +++--- pkg/login/social/connectors/common.go | 10 ++- pkg/login/social/connectors/generic_oauth.go | 76 +++++++++---------- pkg/login/social/connectors/github_oauth.go | 51 ++++++------- pkg/login/social/connectors/gitlab_oauth.go | 23 +++--- pkg/login/social/connectors/google_oauth.go | 30 ++++---- .../social/connectors/google_oauth_test.go | 2 +- .../social/connectors/grafana_com_oauth.go | 11 +-- pkg/login/social/connectors/okta_oauth.go | 29 ++++--- pkg/login/social/connectors/social_base.go | 52 +++++++++++-- 10 files changed, 166 insertions(+), 142 deletions(-) diff --git a/pkg/login/social/connectors/azuread_oauth.go b/pkg/login/social/connectors/azuread_oauth.go index 05f23c706f7..178cc9a837c 100644 --- a/pkg/login/social/connectors/azuread_oauth.go +++ b/pkg/login/social/connectors/azuread_oauth.go @@ -98,6 +98,9 @@ func NewAzureADProvider(info *social.OAuthInfo, cfg *setting.Cfg, ssoSettings ss } func (s *SocialAzureAD) UserInfo(ctx context.Context, client *http.Client, token *oauth2.Token) (*social.BasicUserInfo, error) { + s.reloadMutex.RLock() + defer s.reloadMutex.RUnlock() + idToken := token.Extra("id_token") if idToken == nil { return nil, ErrIDTokenNotFound @@ -118,12 +121,10 @@ func (s *SocialAzureAD) UserInfo(ctx context.Context, client *http.Client, token return nil, ErrEmailNotFound } - info := s.GetOAuthInfo() - // setting the role, grafanaAdmin to empty to reflect that we are not syncronizing with the external provider var role roletype.RoleType var grafanaAdmin bool - if !info.SkipOrgRoleSync { + if !s.info.SkipOrgRoleSync { role, grafanaAdmin, err = s.extractRoleAndAdmin(claims) if err != nil { return nil, err @@ -150,11 +151,11 @@ func (s *SocialAzureAD) UserInfo(ctx context.Context, client *http.Client, token } var isGrafanaAdmin *bool = nil - if info.AllowAssignGrafanaAdmin { + if s.info.AllowAssignGrafanaAdmin { isGrafanaAdmin = &grafanaAdmin } - if info.AllowAssignGrafanaAdmin && info.SkipOrgRoleSync { + if s.info.AllowAssignGrafanaAdmin && s.info.SkipOrgRoleSync { s.log.Debug("AllowAssignGrafanaAdmin and skipOrgRoleSync are both set, Grafana Admin role will not be synced, consider setting one or the other") } @@ -290,10 +291,8 @@ func (claims *azureClaims) extractEmail() string { // extractRoleAndAdmin extracts the role from the claims and returns the role and whether the user is a Grafana admin. func (s *SocialAzureAD) extractRoleAndAdmin(claims *azureClaims) (org.RoleType, bool, error) { - info := s.GetOAuthInfo() - if len(claims.Roles) == 0 { - if info.RoleAttributeStrict { + if s.info.RoleAttributeStrict { return "", false, errRoleAttributeStrictViolation.Errorf("AzureAD OAuth: unset role") } return s.defaultRole(), false, nil @@ -311,7 +310,7 @@ func (s *SocialAzureAD) extractRoleAndAdmin(claims *azureClaims) (org.RoleType, } } - if info.RoleAttributeStrict { + if s.info.RoleAttributeStrict { return "", false, errRoleAttributeStrictViolation.Errorf("AzureAD OAuth: idP did not return a valid role %q", claims.Roles) } @@ -435,15 +434,16 @@ func (s *SocialAzureAD) groupsGraphAPIURL(claims *azureClaims, token *oauth2.Tok } func (s *SocialAzureAD) SupportBundleContent(bf *bytes.Buffer) error { - info := s.GetOAuthInfo() + s.reloadMutex.RLock() + defer s.reloadMutex.RUnlock() bf.WriteString("## AzureAD specific configuration\n\n") bf.WriteString("```ini\n") - bf.WriteString(fmt.Sprintf("allowed_groups = %v\n", info.AllowedGroups)) + bf.WriteString(fmt.Sprintf("allowed_groups = %v\n", s.info.AllowedGroups)) bf.WriteString(fmt.Sprintf("forceUseGraphAPI = %v\n", s.forceUseGraphAPI)) bf.WriteString("```\n\n") - return s.SocialBase.SupportBundleContent(bf) + return s.SocialBase.getBaseSupportBundleContent(bf) } func (s *SocialAzureAD) isAllowedTenant(tenantID string) bool { diff --git a/pkg/login/social/connectors/common.go b/pkg/login/social/connectors/common.go index 6dcc051a319..153e9eb9989 100644 --- a/pkg/login/social/connectors/common.go +++ b/pkg/login/social/connectors/common.go @@ -47,15 +47,17 @@ type httpGetResponse struct { } func (s *SocialBase) IsEmailAllowed(email string) bool { - info := s.GetOAuthInfo() + s.reloadMutex.RLock() + defer s.reloadMutex.RUnlock() - return isEmailAllowed(email, info.AllowedDomains) + return isEmailAllowed(email, s.info.AllowedDomains) } func (s *SocialBase) IsSignupAllowed() bool { - info := s.GetOAuthInfo() + s.reloadMutex.RLock() + defer s.reloadMutex.RUnlock() - return info.AllowSignup + return s.info.AllowSignup } func isEmailAllowed(email string, allowedDomains []string) bool { diff --git a/pkg/login/social/connectors/generic_oauth.go b/pkg/login/social/connectors/generic_oauth.go index 885736e7aea..c76e64d8a01 100644 --- a/pkg/login/social/connectors/generic_oauth.go +++ b/pkg/login/social/connectors/generic_oauth.go @@ -139,14 +139,12 @@ func (s *SocialGenericOAuth) Reload(ctx context.Context, settings ssoModels.SSOS } // TODOD: remove this in the next PR and use the isGroupMember from social.go -func (s *SocialGenericOAuth) IsGroupMember(groups []string) bool { - info := s.GetOAuthInfo() - - if len(info.AllowedGroups) == 0 { +func (s *SocialGenericOAuth) isGroupMember(groups []string) bool { + if len(s.info.AllowedGroups) == 0 { return true } - for _, allowedGroup := range info.AllowedGroups { + for _, allowedGroup := range s.info.AllowedGroups { for _, group := range groups { if group == allowedGroup { return true @@ -157,12 +155,12 @@ func (s *SocialGenericOAuth) IsGroupMember(groups []string) bool { return false } -func (s *SocialGenericOAuth) IsTeamMember(ctx context.Context, client *http.Client) bool { +func (s *SocialGenericOAuth) isTeamMember(ctx context.Context, client *http.Client) bool { if len(s.teamIds) == 0 { return true } - teamMemberships, err := s.FetchTeamMemberships(ctx, client) + teamMemberships, err := s.fetchTeamMemberships(ctx, client) if err != nil { return false } @@ -178,12 +176,12 @@ func (s *SocialGenericOAuth) IsTeamMember(ctx context.Context, client *http.Clie return false } -func (s *SocialGenericOAuth) IsOrganizationMember(ctx context.Context, client *http.Client) bool { +func (s *SocialGenericOAuth) isOrganizationMember(ctx context.Context, client *http.Client) bool { if len(s.allowedOrganizations) == 0 { return true } - organizations, ok := s.FetchOrganizations(ctx, client) + organizations, ok := s.fetchOrganizations(ctx, client) if !ok { return false } @@ -219,6 +217,9 @@ func (info *UserInfoJson) String() string { } func (s *SocialGenericOAuth) UserInfo(ctx context.Context, client *http.Client, token *oauth2.Token) (*social.BasicUserInfo, error) { + s.reloadMutex.RLock() + defer s.reloadMutex.RUnlock() + s.log.Debug("Getting user info") toCheck := make([]*UserInfoJson, 0, 2) @@ -229,8 +230,6 @@ func (s *SocialGenericOAuth) UserInfo(ctx context.Context, client *http.Client, toCheck = append(toCheck, apiData) } - info := s.GetOAuthInfo() - userInfo := &social.BasicUserInfo{} for _, data := range toCheck { s.log.Debug("Processing external user info", "source", data.source, "data", data) @@ -254,13 +253,13 @@ func (s *SocialGenericOAuth) UserInfo(ctx context.Context, client *http.Client, } } - if userInfo.Role == "" && !info.SkipOrgRoleSync { + if userInfo.Role == "" && !s.info.SkipOrgRoleSync { role, grafanaAdmin, err := s.extractRoleAndAdminOptional(data.rawJSON, []string{}) if err != nil { s.log.Warn("Failed to extract role", "err", err) } else { userInfo.Role = role - if info.AllowAssignGrafanaAdmin { + if s.info.AllowAssignGrafanaAdmin { userInfo.IsGrafanaAdmin = &grafanaAdmin } } @@ -277,20 +276,20 @@ func (s *SocialGenericOAuth) UserInfo(ctx context.Context, client *http.Client, } } - if userInfo.Role == "" && !info.SkipOrgRoleSync { - if info.RoleAttributeStrict { + if userInfo.Role == "" && !s.info.SkipOrgRoleSync { + if s.info.RoleAttributeStrict { return nil, errRoleAttributeStrictViolation.Errorf("idP did not return a role attribute") } userInfo.Role = s.defaultRole() } - if info.AllowAssignGrafanaAdmin && info.SkipOrgRoleSync { + if s.info.AllowAssignGrafanaAdmin && s.info.SkipOrgRoleSync { 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 == "" { var err error - userInfo.Email, err = s.FetchPrivateEmail(ctx, client) + userInfo.Email, err = s.fetchPrivateEmail(ctx, client) if err != nil { return nil, err } @@ -302,15 +301,15 @@ func (s *SocialGenericOAuth) UserInfo(ctx context.Context, client *http.Client, userInfo.Login = userInfo.Email } - if !s.IsTeamMember(ctx, client) { + if !s.isTeamMember(ctx, client) { return nil, errors.New("user not a member of one of the required teams") } - if !s.IsOrganizationMember(ctx, client) { + if !s.isOrganizationMember(ctx, client) { return nil, errors.New("user not a member of one of the required organizations") } - if !s.IsGroupMember(userInfo.Groups) { + if !s.isGroupMember(userInfo.Groups) { return nil, errMissingGroupMembership } @@ -352,17 +351,15 @@ func (s *SocialGenericOAuth) extractFromToken(token *oauth2.Token) *UserInfoJson } func (s *SocialGenericOAuth) extractFromAPI(ctx context.Context, client *http.Client) *UserInfoJson { - info := s.GetOAuthInfo() - s.log.Debug("Getting user info from API") - if info.ApiUrl == "" { + if s.info.ApiUrl == "" { s.log.Debug("No api url configured") return nil } - rawUserInfoResponse, err := s.httpGet(ctx, client, info.ApiUrl) + rawUserInfoResponse, err := s.httpGet(ctx, client, s.info.ApiUrl) if err != nil { - s.log.Debug("Error getting user info from API", "url", info.ApiUrl, "error", err) + s.log.Debug("Error getting user info from API", "url", s.info.ApiUrl, "error", err) return nil } @@ -469,7 +466,7 @@ func (s *SocialGenericOAuth) extractGroups(data *UserInfoJson) ([]string, error) return util.SearchJSONForStringSliceAttr(s.groupsAttributePath, data.rawJSON) } -func (s *SocialGenericOAuth) FetchPrivateEmail(ctx context.Context, client *http.Client) (string, error) { +func (s *SocialGenericOAuth) fetchPrivateEmail(ctx context.Context, client *http.Client) (string, error) { type Record struct { Email string `json:"email"` Primary bool `json:"primary"` @@ -478,11 +475,9 @@ func (s *SocialGenericOAuth) FetchPrivateEmail(ctx context.Context, client *http IsConfirmed bool `json:"is_confirmed"` } - info := s.GetOAuthInfo() - - response, err := s.httpGet(ctx, client, fmt.Sprintf(info.ApiUrl+"/emails")) + response, err := s.httpGet(ctx, client, fmt.Sprintf(s.info.ApiUrl+"/emails")) if err != nil { - s.log.Error("Error getting email address", "url", info.ApiUrl+"/emails", "error", err) + s.log.Error("Error getting email address", "url", s.info.ApiUrl+"/emails", "error", err) return "", fmt.Errorf("%v: %w", "Error getting email address", err) } @@ -518,7 +513,7 @@ func (s *SocialGenericOAuth) FetchPrivateEmail(ctx context.Context, client *http return email, nil } -func (s *SocialGenericOAuth) FetchTeamMemberships(ctx context.Context, client *http.Client) ([]string, error) { +func (s *SocialGenericOAuth) fetchTeamMemberships(ctx context.Context, client *http.Client) ([]string, error) { var err error var ids []string @@ -542,11 +537,9 @@ func (s *SocialGenericOAuth) fetchTeamMembershipsFromDeprecatedTeamsUrl(ctx cont Id int `json:"id"` } - info := s.GetOAuthInfo() - - response, err := s.httpGet(ctx, client, fmt.Sprintf(info.ApiUrl+"/teams")) + response, err := s.httpGet(ctx, client, fmt.Sprintf(s.info.ApiUrl+"/teams")) if err != nil { - s.log.Error("Error getting team memberships", "url", info.ApiUrl+"/teams", "error", err) + s.log.Error("Error getting team memberships", "url", s.info.ApiUrl+"/teams", "error", err) return []string{}, err } @@ -580,16 +573,14 @@ func (s *SocialGenericOAuth) fetchTeamMembershipsFromTeamsUrl(ctx context.Contex return util.SearchJSONForStringSliceAttr(s.teamIdsAttributePath, response.Body) } -func (s *SocialGenericOAuth) FetchOrganizations(ctx context.Context, client *http.Client) ([]string, bool) { +func (s *SocialGenericOAuth) fetchOrganizations(ctx context.Context, client *http.Client) ([]string, bool) { type Record struct { Login string `json:"login"` } - info := s.GetOAuthInfo() - - response, err := s.httpGet(ctx, client, fmt.Sprintf(info.ApiUrl+"/orgs")) + response, err := s.httpGet(ctx, client, fmt.Sprintf(s.info.ApiUrl+"/orgs")) if err != nil { - s.log.Error("Error getting organizations", "url", info.ApiUrl+"/orgs", "error", err) + s.log.Error("Error getting organizations", "url", s.info.ApiUrl+"/orgs", "error", err) return nil, false } @@ -612,6 +603,9 @@ func (s *SocialGenericOAuth) FetchOrganizations(ctx context.Context, client *htt } func (s *SocialGenericOAuth) SupportBundleContent(bf *bytes.Buffer) error { + s.reloadMutex.RLock() + defer s.reloadMutex.RUnlock() + bf.WriteString("## GenericOAuth specific configuration\n\n") bf.WriteString("```ini\n") bf.WriteString(fmt.Sprintf("name_attribute_path = %s\n", s.nameAttributePath)) @@ -622,5 +616,5 @@ func (s *SocialGenericOAuth) SupportBundleContent(bf *bytes.Buffer) error { bf.WriteString(fmt.Sprintf("allowed_organizations = %v\n", s.allowedOrganizations)) bf.WriteString("```\n\n") - return s.SocialBase.SupportBundleContent(bf) + return s.SocialBase.getBaseSupportBundleContent(bf) } diff --git a/pkg/login/social/connectors/github_oauth.go b/pkg/login/social/connectors/github_oauth.go index 46a82de22a8..18e2175bcc1 100644 --- a/pkg/login/social/connectors/github_oauth.go +++ b/pkg/login/social/connectors/github_oauth.go @@ -132,12 +132,12 @@ func (s *SocialGithub) Reload(ctx context.Context, settings ssoModels.SSOSetting return nil } -func (s *SocialGithub) IsTeamMember(ctx context.Context, client *http.Client) bool { +func (s *SocialGithub) isTeamMember(ctx context.Context, client *http.Client) bool { if len(s.teamIds) == 0 { return true } - teamMemberships, err := s.FetchTeamMemberships(ctx, client) + teamMemberships, err := s.fetchTeamMemberships(ctx, client) if err != nil { return false } @@ -153,13 +153,13 @@ func (s *SocialGithub) IsTeamMember(ctx context.Context, client *http.Client) bo return false } -func (s *SocialGithub) IsOrganizationMember(ctx context.Context, +func (s *SocialGithub) isOrganizationMember(ctx context.Context, client *http.Client, organizationsUrl string) bool { if len(s.allowedOrganizations) == 0 { return true } - organizations, err := s.FetchOrganizations(ctx, client, organizationsUrl) + organizations, err := s.fetchOrganizations(ctx, client, organizationsUrl) if err != nil { return false } @@ -175,16 +175,14 @@ func (s *SocialGithub) IsOrganizationMember(ctx context.Context, return false } -func (s *SocialGithub) FetchPrivateEmail(ctx context.Context, client *http.Client) (string, error) { +func (s *SocialGithub) fetchPrivateEmail(ctx context.Context, client *http.Client) (string, error) { type Record struct { Email string `json:"email"` Primary bool `json:"primary"` Verified bool `json:"verified"` } - info := s.GetOAuthInfo() - - response, err := s.httpGet(ctx, client, fmt.Sprintf(info.ApiUrl+"/emails")) + response, err := s.httpGet(ctx, client, fmt.Sprintf(s.info.ApiUrl+"/emails")) if err != nil { return "", fmt.Errorf("Error getting email address: %s", err) } @@ -206,10 +204,8 @@ func (s *SocialGithub) FetchPrivateEmail(ctx context.Context, client *http.Clien return email, nil } -func (s *SocialGithub) FetchTeamMemberships(ctx context.Context, client *http.Client) ([]GithubTeam, error) { - info := s.GetOAuthInfo() - - url := fmt.Sprintf(info.ApiUrl + "/teams?per_page=100") +func (s *SocialGithub) fetchTeamMemberships(ctx context.Context, client *http.Client) ([]GithubTeam, error) { + url := fmt.Sprintf(s.info.ApiUrl + "/teams?per_page=100") hasMore := true teams := make([]GithubTeam, 0) @@ -228,13 +224,13 @@ func (s *SocialGithub) FetchTeamMemberships(ctx context.Context, client *http.Cl teams = append(teams, records...) - url, hasMore = s.HasMoreRecords(response.Headers) + url, hasMore = s.hasMoreRecords(response.Headers) } return teams, nil } -func (s *SocialGithub) HasMoreRecords(headers http.Header) (string, bool) { +func (s *SocialGithub) hasMoreRecords(headers http.Header) (string, bool) { value, exists := headers["Link"] if !exists { return "", false @@ -252,7 +248,7 @@ func (s *SocialGithub) HasMoreRecords(headers http.Header) (string, bool) { return url, true } -func (s *SocialGithub) FetchOrganizations(ctx context.Context, client *http.Client, organizationsUrl string) ([]string, error) { +func (s *SocialGithub) fetchOrganizations(ctx context.Context, client *http.Client, organizationsUrl string) ([]string, error) { url := organizationsUrl hasMore := true logins := make([]string, 0) @@ -278,12 +274,15 @@ func (s *SocialGithub) FetchOrganizations(ctx context.Context, client *http.Clie logins = append(logins, record.Login) } - url, hasMore = s.HasMoreRecords(response.Headers) + url, hasMore = s.hasMoreRecords(response.Headers) } return logins, nil } func (s *SocialGithub) UserInfo(ctx context.Context, client *http.Client, token *oauth2.Token) (*social.BasicUserInfo, error) { + s.reloadMutex.RLock() + defer s.reloadMutex.RUnlock() + var data struct { Id int `json:"id"` Login string `json:"login"` @@ -291,9 +290,7 @@ func (s *SocialGithub) UserInfo(ctx context.Context, client *http.Client, token Name string `json:"name"` } - info := s.GetOAuthInfo() - - response, err := s.httpGet(ctx, client, info.ApiUrl) + response, err := s.httpGet(ctx, client, s.info.ApiUrl) if err != nil { return nil, fmt.Errorf("error getting user info: %s", err) } @@ -302,7 +299,7 @@ func (s *SocialGithub) UserInfo(ctx context.Context, client *http.Client, token return nil, fmt.Errorf("error unmarshalling user info: %s", err) } - teamMemberships, err := s.FetchTeamMemberships(ctx, client) + teamMemberships, err := s.fetchTeamMemberships(ctx, client) if err != nil { return nil, fmt.Errorf("error getting user teams: %s", err) } @@ -312,20 +309,20 @@ func (s *SocialGithub) UserInfo(ctx context.Context, client *http.Client, token var role roletype.RoleType var isGrafanaAdmin *bool = nil - if !info.SkipOrgRoleSync { + if !s.info.SkipOrgRoleSync { var grafanaAdmin bool role, grafanaAdmin, err = s.extractRoleAndAdmin(response.Body, teams) if err != nil { return nil, err } - if info.AllowAssignGrafanaAdmin { + if s.info.AllowAssignGrafanaAdmin { isGrafanaAdmin = &grafanaAdmin } } // we skip allowing assignment of GrafanaAdmin if skipOrgRoleSync is present - if info.AllowAssignGrafanaAdmin && info.SkipOrgRoleSync { + if s.info.AllowAssignGrafanaAdmin && s.info.SkipOrgRoleSync { s.log.Debug("AllowAssignGrafanaAdmin and skipOrgRoleSync are both set, Grafana Admin role will not be synced, consider setting one or the other") } @@ -342,20 +339,20 @@ func (s *SocialGithub) UserInfo(ctx context.Context, client *http.Client, token userInfo.Name = data.Name } - organizationsUrl := fmt.Sprintf(info.ApiUrl + "/orgs?per_page=100") + organizationsUrl := fmt.Sprintf(s.info.ApiUrl + "/orgs?per_page=100") - if !s.IsTeamMember(ctx, client) { + if !s.isTeamMember(ctx, client) { return nil, ErrMissingTeamMembership.Errorf("User is not a member of any of the allowed teams: %v", s.teamIds) } - if !s.IsOrganizationMember(ctx, client, organizationsUrl) { + if !s.isOrganizationMember(ctx, client, organizationsUrl) { return nil, ErrMissingOrganizationMembership.Errorf( "User is not a member of any of the allowed organizations: %v", s.allowedOrganizations) } if userInfo.Email == "" { - userInfo.Email, err = s.FetchPrivateEmail(ctx, client) + userInfo.Email, err = s.fetchPrivateEmail(ctx, client) if err != nil { return nil, err } diff --git a/pkg/login/social/connectors/gitlab_oauth.go b/pkg/login/social/connectors/gitlab_oauth.go index 258edde7d47..b588d2c2ebf 100644 --- a/pkg/login/social/connectors/gitlab_oauth.go +++ b/pkg/login/social/connectors/gitlab_oauth.go @@ -116,9 +116,7 @@ func (s *SocialGitlab) getGroupsPage(ctx context.Context, client *http.Client, n FullPath string `json:"full_path"` } - info := s.GetOAuthInfo() - - groupURL, err := url.JoinPath(info.ApiUrl, "/groups") + groupURL, err := url.JoinPath(s.info.ApiUrl, "/groups") if err != nil { s.log.Error("Error joining GitLab API URL", "err", err) return nil, nil @@ -179,7 +177,8 @@ func (s *SocialGitlab) getGroupsPage(ctx context.Context, client *http.Client, n } func (s *SocialGitlab) UserInfo(ctx context.Context, client *http.Client, token *oauth2.Token) (*social.BasicUserInfo, error) { - info := s.GetOAuthInfo() + s.reloadMutex.RLock() + defer s.reloadMutex.RUnlock() data, err := s.extractFromToken(ctx, client, token) if err != nil { @@ -209,7 +208,7 @@ func (s *SocialGitlab) UserInfo(ctx context.Context, client *http.Client, token return nil, errMissingGroupMembership } - if info.AllowAssignGrafanaAdmin && info.SkipOrgRoleSync { + if s.info.AllowAssignGrafanaAdmin && s.info.SkipOrgRoleSync { s.log.Debug("AllowAssignGrafanaAdmin and skipOrgRoleSync are both set, Grafana Admin role will not be synced, consider setting one or the other") } @@ -217,10 +216,8 @@ func (s *SocialGitlab) UserInfo(ctx context.Context, client *http.Client, token } func (s *SocialGitlab) extractFromAPI(ctx context.Context, client *http.Client, token *oauth2.Token) (*userData, error) { - info := s.GetOAuthInfo() - apiResp := &apiData{} - response, err := s.httpGet(ctx, client, info.ApiUrl+"/user") + response, err := s.httpGet(ctx, client, s.info.ApiUrl+"/user") if err != nil { return nil, fmt.Errorf("Error getting user info: %w", err) } @@ -246,14 +243,14 @@ func (s *SocialGitlab) extractFromAPI(ctx context.Context, client *http.Client, Groups: s.getGroups(ctx, client), } - if !info.SkipOrgRoleSync { + if !s.info.SkipOrgRoleSync { var grafanaAdmin bool role, grafanaAdmin, err := s.extractRoleAndAdmin(response.Body, idData.Groups) if err != nil { return nil, err } - if info.AllowAssignGrafanaAdmin { + if s.info.AllowAssignGrafanaAdmin { idData.IsGrafanaAdmin = &grafanaAdmin } @@ -270,8 +267,6 @@ func (s *SocialGitlab) extractFromAPI(ctx context.Context, client *http.Client, func (s *SocialGitlab) extractFromToken(ctx context.Context, client *http.Client, token *oauth2.Token) (*userData, error) { s.log.Debug("Extracting user info from OAuth token") - info := s.GetOAuthInfo() - idToken := token.Extra("id_token") if idToken == nil { s.log.Debug("No id_token found, defaulting to API access", "token", token) @@ -305,13 +300,13 @@ func (s *SocialGitlab) extractFromToken(ctx context.Context, client *http.Client data.Groups = userInfo.Groups } - if !info.SkipOrgRoleSync { + if !s.info.SkipOrgRoleSync { role, grafanaAdmin, errRole := s.extractRoleAndAdmin(rawJSON, data.Groups) if errRole != nil { return nil, errRole } - if info.AllowAssignGrafanaAdmin { + if s.info.AllowAssignGrafanaAdmin { data.IsGrafanaAdmin = &grafanaAdmin } diff --git a/pkg/login/social/connectors/google_oauth.go b/pkg/login/social/connectors/google_oauth.go index dabf0f25f58..0bda73cd62a 100644 --- a/pkg/login/social/connectors/google_oauth.go +++ b/pkg/login/social/connectors/google_oauth.go @@ -102,7 +102,8 @@ func (s *SocialGoogle) Reload(ctx context.Context, settings ssoModels.SSOSetting } func (s *SocialGoogle) UserInfo(ctx context.Context, client *http.Client, token *oauth2.Token) (*social.BasicUserInfo, error) { - info := s.GetOAuthInfo() + s.reloadMutex.RLock() + defer s.reloadMutex.RUnlock() data, errToken := s.extractFromToken(ctx, client, token) if errToken != nil { @@ -125,7 +126,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, info); err != nil { + if err := s.isHDAllowed(data.HD); err != nil { return nil, err } @@ -148,13 +149,13 @@ func (s *SocialGoogle) UserInfo(ctx context.Context, client *http.Client, token Groups: groups, } - if !info.SkipOrgRoleSync { + if !s.info.SkipOrgRoleSync { role, grafanaAdmin, errRole := s.extractRoleAndAdmin(data.rawJSON, groups) if errRole != nil { return nil, errRole } - if info.AllowAssignGrafanaAdmin { + if s.info.AllowAssignGrafanaAdmin { userInfo.IsGrafanaAdmin = &grafanaAdmin } @@ -175,11 +176,9 @@ type googleAPIData struct { } func (s *SocialGoogle) extractFromAPI(ctx context.Context, client *http.Client) (*googleUserData, error) { - info := s.GetOAuthInfo() - - if strings.HasPrefix(info.ApiUrl, legacyAPIURL) { + if strings.HasPrefix(s.info.ApiUrl, legacyAPIURL) { data := googleAPIData{} - response, err := s.httpGet(ctx, client, info.ApiUrl) + response, err := s.httpGet(ctx, client, s.info.ApiUrl) if err != nil { return nil, fmt.Errorf("error retrieving legacy user info: %s", err) } @@ -199,7 +198,7 @@ func (s *SocialGoogle) extractFromAPI(ctx context.Context, client *http.Client) } data := googleUserData{} - response, err := s.httpGet(ctx, client, info.ApiUrl) + response, err := s.httpGet(ctx, client, s.info.ApiUrl) if err != nil { return nil, fmt.Errorf("error getting user info: %s", err) } @@ -212,12 +211,13 @@ func (s *SocialGoogle) extractFromAPI(ctx context.Context, client *http.Client) } func (s *SocialGoogle) AuthCodeURL(state string, opts ...oauth2.AuthCodeOption) string { - info := s.GetOAuthInfo() + s.reloadMutex.RLock() + defer s.reloadMutex.RUnlock() - if info.UseRefreshToken { + if s.info.UseRefreshToken { opts = append(opts, oauth2.AccessTypeOffline, oauth2.ApprovalForce) } - return s.SocialBase.AuthCodeURL(state, opts...) + return s.SocialBase.Config.AuthCodeURL(state, opts...) } func (s *SocialGoogle) extractFromToken(ctx context.Context, client *http.Client, token *oauth2.Token) (*googleUserData, error) { @@ -307,16 +307,16 @@ func (s *SocialGoogle) getGroupsPage(ctx context.Context, client *http.Client, u return &data, nil } -func (s *SocialGoogle) isHDAllowed(hd string, info *social.OAuthInfo) error { +func (s *SocialGoogle) isHDAllowed(hd string) error { if s.validateHD { return nil } - if len(info.AllowedDomains) == 0 { + if len(s.info.AllowedDomains) == 0 { return nil } - for _, allowedDomain := range info.AllowedDomains { + for _, allowedDomain := range s.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 d822d4d7b88..64b40f59130 100644 --- a/pkg/login/social/connectors/google_oauth_test.go +++ b/pkg/login/social/connectors/google_oauth_test.go @@ -931,7 +931,7 @@ func TestIsHDAllowed(t *testing.T) { info.AllowedDomains = tc.allowedDomains s := NewGoogleProvider(info, &setting.Cfg{}, &ssosettingstests.MockService{}, featuremgmt.WithFeatures()) s.validateHD = tc.validateHD - err := s.isHDAllowed(tc.email, info) + err := s.isHDAllowed(tc.email) if tc.expectedErrorMessage != "" { require.Error(t, err) diff --git a/pkg/login/social/connectors/grafana_com_oauth.go b/pkg/login/social/connectors/grafana_com_oauth.go index 694400aea47..c68ecb2d8e8 100644 --- a/pkg/login/social/connectors/grafana_com_oauth.go +++ b/pkg/login/social/connectors/grafana_com_oauth.go @@ -99,7 +99,7 @@ func (s *SocialGrafanaCom) IsEmailAllowed(email string) bool { return true } -func (s *SocialGrafanaCom) IsOrganizationMember(organizations []OrgRecord) bool { +func (s *SocialGrafanaCom) isOrganizationMember(organizations []OrgRecord) bool { if len(s.allowedOrganizations) == 0 { return true } @@ -117,6 +117,9 @@ func (s *SocialGrafanaCom) IsOrganizationMember(organizations []OrgRecord) bool // UserInfo is used for login credentials for the user func (s *SocialGrafanaCom) UserInfo(ctx context.Context, client *http.Client, _ *oauth2.Token) (*social.BasicUserInfo, error) { + s.reloadMutex.RLock() + defer s.reloadMutex.RUnlock() + var data struct { Id int `json:"id"` Name string `json:"name"` @@ -126,8 +129,6 @@ func (s *SocialGrafanaCom) UserInfo(ctx context.Context, client *http.Client, _ Orgs []OrgRecord `json:"orgs"` } - info := s.GetOAuthInfo() - response, err := s.httpGet(ctx, client, s.url+"/api/oauth2/user") if err != nil { @@ -141,7 +142,7 @@ func (s *SocialGrafanaCom) UserInfo(ctx context.Context, client *http.Client, _ // on login we do not want to display the role from the external provider var role roletype.RoleType - if !info.SkipOrgRoleSync { + if !s.info.SkipOrgRoleSync { role = org.RoleType(data.Role) } userInfo := &social.BasicUserInfo{ @@ -152,7 +153,7 @@ func (s *SocialGrafanaCom) UserInfo(ctx context.Context, client *http.Client, _ Role: role, } - if !s.IsOrganizationMember(data.Orgs) { + if !s.isOrganizationMember(data.Orgs) { return nil, ErrMissingOrganizationMembership.Errorf( "User is not a member of any of the allowed organizations: %v. Returned Organizations: %v", s.allowedOrganizations, data.Orgs) diff --git a/pkg/login/social/connectors/okta_oauth.go b/pkg/login/social/connectors/okta_oauth.go index 6448b26b748..d548827c83c 100644 --- a/pkg/login/social/connectors/okta_oauth.go +++ b/pkg/login/social/connectors/okta_oauth.go @@ -105,7 +105,8 @@ func (claims *OktaClaims) extractEmail() string { } func (s *SocialOkta) UserInfo(ctx context.Context, client *http.Client, token *oauth2.Token) (*social.BasicUserInfo, error) { - info := s.GetOAuthInfo() + s.reloadMutex.RLock() + defer s.reloadMutex.RUnlock() idToken := token.Extra("id_token") if idToken == nil { @@ -133,25 +134,25 @@ func (s *SocialOkta) UserInfo(ctx context.Context, client *http.Client, token *o return nil, err } - groups := s.GetGroups(&data) - if !s.IsGroupMember(groups) { + groups := s.getGroups(&data) + if !s.isGroupMember(groups) { return nil, errMissingGroupMembership } var role roletype.RoleType var isGrafanaAdmin *bool - if !info.SkipOrgRoleSync { + if !s.info.SkipOrgRoleSync { var grafanaAdmin bool role, grafanaAdmin, err = s.extractRoleAndAdmin(data.rawJSON, groups) if err != nil { return nil, err } - if info.AllowAssignGrafanaAdmin { + if s.info.AllowAssignGrafanaAdmin { isGrafanaAdmin = &grafanaAdmin } } - if info.AllowAssignGrafanaAdmin && info.SkipOrgRoleSync { + if s.info.AllowAssignGrafanaAdmin && s.info.SkipOrgRoleSync { s.log.Debug("AllowAssignGrafanaAdmin and skipOrgRoleSync are both set, Grafana Admin role will not be synced, consider setting one or the other") } @@ -167,11 +168,9 @@ func (s *SocialOkta) UserInfo(ctx context.Context, client *http.Client, token *o } func (s *SocialOkta) extractAPI(ctx context.Context, data *OktaUserInfoJson, client *http.Client) error { - info := s.GetOAuthInfo() - - rawUserInfoResponse, err := s.httpGet(ctx, client, info.ApiUrl) + rawUserInfoResponse, err := s.httpGet(ctx, client, s.info.ApiUrl) if err != nil { - s.log.Debug("Error getting user info response", "url", info.ApiUrl, "error", err) + s.log.Debug("Error getting user info response", "url", s.info.ApiUrl, "error", err) return fmt.Errorf("error getting user info response: %w", err) } data.rawJSON = rawUserInfoResponse.Body @@ -187,7 +186,7 @@ func (s *SocialOkta) extractAPI(ctx context.Context, data *OktaUserInfoJson, cli return nil } -func (s *SocialOkta) GetGroups(data *OktaUserInfoJson) []string { +func (s *SocialOkta) getGroups(data *OktaUserInfoJson) []string { groups := make([]string, 0) if len(data.Groups) > 0 { groups = data.Groups @@ -196,14 +195,12 @@ func (s *SocialOkta) GetGroups(data *OktaUserInfoJson) []string { } // TODO: remove this in a separate PR and use the isGroupMember from the social.go -func (s *SocialOkta) IsGroupMember(groups []string) bool { - info := s.GetOAuthInfo() - - if len(info.AllowedGroups) == 0 { +func (s *SocialOkta) isGroupMember(groups []string) bool { + if len(s.info.AllowedGroups) == 0 { return true } - for _, allowedGroup := range info.AllowedGroups { + for _, allowedGroup := range s.info.AllowedGroups { for _, group := range groups { if group == allowedGroup { return true diff --git a/pkg/login/social/connectors/social_base.go b/pkg/login/social/connectors/social_base.go index 577bdc41e5e..6e53c13e769 100644 --- a/pkg/login/social/connectors/social_base.go +++ b/pkg/login/social/connectors/social_base.go @@ -3,10 +3,12 @@ package connectors import ( "bytes" "compress/zlib" + "context" "encoding/base64" "encoding/json" "fmt" "io" + "net/http" "regexp" "strings" "sync" @@ -60,6 +62,48 @@ type groupStruct struct { } func (s *SocialBase) SupportBundleContent(bf *bytes.Buffer) error { + s.reloadMutex.RLock() + defer s.reloadMutex.RUnlock() + + return s.getBaseSupportBundleContent(bf) +} + +func (s *SocialBase) GetOAuthInfo() *social.OAuthInfo { + s.reloadMutex.RLock() + defer s.reloadMutex.RUnlock() + + return s.info +} + +func (s *SocialBase) AuthCodeURL(state string, opts ...oauth2.AuthCodeOption) string { + s.reloadMutex.RLock() + defer s.reloadMutex.RUnlock() + + return s.Config.AuthCodeURL(state, opts...) +} + +func (s *SocialBase) Exchange(ctx context.Context, code string, opts ...oauth2.AuthCodeOption) (*oauth2.Token, error) { + s.reloadMutex.RLock() + defer s.reloadMutex.RUnlock() + + return s.Config.Exchange(ctx, code, opts...) +} + +func (s *SocialBase) Client(ctx context.Context, t *oauth2.Token) *http.Client { + s.reloadMutex.RLock() + defer s.reloadMutex.RUnlock() + + return s.Config.Client(ctx, t) +} + +func (s *SocialBase) TokenSource(ctx context.Context, t *oauth2.Token) oauth2.TokenSource { + s.reloadMutex.RLock() + defer s.reloadMutex.RUnlock() + + return s.Config.TokenSource(ctx, t) +} + +func (s *SocialBase) getBaseSupportBundleContent(bf *bytes.Buffer) error { bf.WriteString("## Client configuration\n\n") bf.WriteString("```ini\n") bf.WriteString(fmt.Sprintf("allow_assign_grafana_admin = %v\n", s.info.AllowAssignGrafanaAdmin)) @@ -77,16 +121,10 @@ func (s *SocialBase) SupportBundleContent(bf *bytes.Buffer) error { bf.WriteString(fmt.Sprintf("redirect_url = %v\n", s.Config.RedirectURL)) bf.WriteString(fmt.Sprintf("scopes = %v\n", s.Config.Scopes)) bf.WriteString("```\n\n") + return nil } -func (s *SocialBase) GetOAuthInfo() *social.OAuthInfo { - s.reloadMutex.RLock() - defer s.reloadMutex.RUnlock() - - return s.info -} - func (s *SocialBase) extractRoleAndAdminOptional(rawJSON []byte, groups []string) (org.RoleType, bool, error) { if s.info.RoleAttributePath == "" { if s.info.RoleAttributeStrict {